| Message ID | 20190815083716.4715-2-xiaowei.bao@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/10] PCI: designware-ep: Add multiple PFs support for DWC | expand |
On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote: > Add the doorbell mode of MSI-X in EP mode. > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> > --- > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++ > drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 75e2955..e3a7cdf 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > return 0; > } > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, > + u16 interrupt_num) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + u32 msg_data; > + > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | > + (interrupt_num - 1); > + > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); > + > + return 0; > +} > + > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > u16 interrupt_num) Have I understood correctly that the hardware provides an alternative mechanism that allows for raising MSI-X interrupts without the bother of reading the capabilities registers? If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus use it in dw_plat_pcie_ep_raise_irq also)? > { > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 2b291e8..cd903e9 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -88,6 +88,11 @@ > #define PCIE_MISC_CONTROL_1_OFF 0x8BC > #define PCIE_DBI_RO_WR_EN BIT(0) > > +#define PCIE_MSIX_DOORBELL 0x948 > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24 > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16 > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15) The _VF defines are not used, I'd suggest removing them. Thanks, Andrew Murray > + > /* > * iATU Unroll-specific register definitions > * From 4.80 core version the address translation will be made by unroll > @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, > u8 interrupt_num); > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > u16 interrupt_num); > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, > + u16 interrupt_num); > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar); > #else > static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) > @@ -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > return 0; > } > > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, > + u8 func_no, > + u16 interrupt_num) > +{ > + return 0; > +} > + > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) > { > } > -- > 2.9.5 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> -----Original Message----- > From: Andrew Murray <andrew.murray@arm.com> > Sent: 2019年8月15日 19:54 > To: Xiaowei Bao <xiaowei.bao@nxp.com> > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; > lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org; > M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; > Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org > Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of > MSI-X in EP mode > > On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote: > > Add the doorbell mode of MSI-X in EP mode. > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> > > --- > > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++ > > drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++ > > 2 files changed, 28 insertions(+) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > > b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 75e2955..e3a7cdf 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep > *ep, u8 func_no, > > return 0; > > } > > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > func_no, > > + u16 interrupt_num) > > +{ > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > + u32 msg_data; > > + > > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | > > + (interrupt_num - 1); > > + > > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); > > + > > + return 0; > > +} > > + > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > u16 interrupt_num) > > Have I understood correctly that the hardware provides an alternative > mechanism that allows for raising MSI-X interrupts without the bother of > reading the capabilities registers? Yes, the hardware provide two way to MSI-X, please check the page 492 of DWC_pcie_dm_registers_4.30 Menu. MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....> > > If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus > use it in dw_plat_pcie_ep_raise_irq also)? I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct, because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr, but I also don't know where I can get the correct MSIX table. > > > > { > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h > > b/drivers/pci/controller/dwc/pcie-designware.h > > index 2b291e8..cd903e9 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > @@ -88,6 +88,11 @@ > > #define PCIE_MISC_CONTROL_1_OFF 0x8BC > > #define PCIE_DBI_RO_WR_EN BIT(0) > > > > +#define PCIE_MSIX_DOORBELL 0x948 > > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24 > > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16 > > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15) > > The _VF defines are not used, I'd suggest removing them. In fact, I will add the SRIOV support in this file, the SRIOV feature have verified In my board, but I need wait the EP framework SRIOV patch merge, so I defined these two macros. > > Thanks, > > Andrew Murray > > > + > > /* > > * iATU Unroll-specific register definitions > > * From 4.80 core version the address translation will be made by > > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct > dw_pcie_ep *ep, u8 func_no, > > u8 interrupt_num); > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > u16 interrupt_num); > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > func_no, > > + u16 interrupt_num); > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar); > > #else static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@ > > -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct > dw_pcie_ep *ep, u8 func_no, > > return 0; > > } > > > > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep > *ep, > > + u8 func_no, > > + u16 interrupt_num) > > +{ > > + return 0; > > +} > > + > > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum > > pci_barno bar) { } > > -- > > 2.9.5 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02% > 7C0 > > > 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972% > 7C686ea1d > > > 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&sd > ata=dyrXB > > > avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&reserved=0
On Fri, Aug 16, 2019 at 02:58:31AM +0000, Xiaowei Bao wrote: > > > > -----Original Message----- > > From: Andrew Murray <andrew.murray@arm.com> > > Sent: 2019年8月15日 19:54 > > To: Xiaowei Bao <xiaowei.bao@nxp.com> > > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; > > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; > > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; > > lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org; > > M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; > > Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org; > > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > > linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org > > Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of > > MSI-X in EP mode > > > > On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote: > > > Add the doorbell mode of MSI-X in EP mode. > > > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> > > > --- > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++ > > > drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++ > > > 2 files changed, 28 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > index 75e2955..e3a7cdf 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep > > *ep, u8 func_no, > > > return 0; > > > } > > > > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > > func_no, > > > + u16 interrupt_num) > > > +{ > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > > + u32 msg_data; > > > + > > > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | > > > + (interrupt_num - 1); > > > + > > > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); > > > + > > > + return 0; > > > +} > > > + > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > > u16 interrupt_num) > > > > Have I understood correctly that the hardware provides an alternative > > mechanism that allows for raising MSI-X interrupts without the bother of > > reading the capabilities registers? > Yes, the hardware provide two way to MSI-X, please check the page 492 of > DWC_pcie_dm_registers_4.30 Menu. > MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....> Thanks for the reference. > > > > If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus > > use it in dw_plat_pcie_ep_raise_irq also)? > I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct, > because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr, > but I also don't know where I can get the correct MSIX table. Well it looks like this function is used by snps,dw-pcie-ep and snps,dw-pcie, perhaps the doorbell mode isn't available on that hardware. > > > > > > > { > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h > > > b/drivers/pci/controller/dwc/pcie-designware.h > > > index 2b291e8..cd903e9 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > > @@ -88,6 +88,11 @@ > > > #define PCIE_MISC_CONTROL_1_OFF 0x8BC > > > #define PCIE_DBI_RO_WR_EN BIT(0) > > > > > > +#define PCIE_MSIX_DOORBELL 0x948 > > > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24 > > > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16 > > > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15) > > > > The _VF defines are not used, I'd suggest removing them. > In fact, I will add the SRIOV support in this file, the SRIOV feature have verified > In my board, but I need wait the EP framework SRIOV patch merge, > so I defined these two macros. I'd suggest adding the VF macros along with the SRIOV feature. Thanks, Andrew Murray > > > > Thanks, > > > > Andrew Murray > > > > > + > > > /* > > > * iATU Unroll-specific register definitions > > > * From 4.80 core version the address translation will be made by > > > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct > > dw_pcie_ep *ep, u8 func_no, > > > u8 interrupt_num); > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > > u16 interrupt_num); > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > > func_no, > > > + u16 interrupt_num); > > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar); > > > #else static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@ > > > -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct > > dw_pcie_ep *ep, u8 func_no, > > > return 0; > > > } > > > > > > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep > > *ep, > > > + u8 func_no, > > > + u16 interrupt_num) > > > +{ > > > + return 0; > > > +} > > > + > > > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum > > > pci_barno bar) { } > > > -- > > > 2.9.5 > > > > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists > > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02% > > 7C0 > > > > > 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972% > > 7C686ea1d > > > > > 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&sd > > ata=dyrXB > > > > > avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&reserved=0
Hi, On 16/08/19 8:28 AM, Xiaowei Bao wrote: > > >> -----Original Message----- >> From: Andrew Murray <andrew.murray@arm.com> >> Sent: 2019年8月15日 19:54 >> To: Xiaowei Bao <xiaowei.bao@nxp.com> >> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; >> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; >> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; >> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org; >> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; >> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org; >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; >> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org >> Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of >> MSI-X in EP mode >> >> On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote: >>> Add the doorbell mode of MSI-X in EP mode. >>> >>> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> >>> --- >>> drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++ >>> drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++ >>> 2 files changed, 28 insertions(+) >>> >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c >>> b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> index 75e2955..e3a7cdf 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep >> *ep, u8 func_no, >>> return 0; >>> } >>> >>> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 >> func_no, >>> + u16 interrupt_num) >>> +{ >>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); >>> + u32 msg_data; >>> + >>> + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | >>> + (interrupt_num - 1); >>> + >>> + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); >>> + >>> + return 0; >>> +} >>> + >>> int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, >>> u16 interrupt_num) >> >> Have I understood correctly that the hardware provides an alternative >> mechanism that allows for raising MSI-X interrupts without the bother of >> reading the capabilities registers? > Yes, the hardware provide two way to MSI-X, please check the page 492 of > DWC_pcie_dm_registers_4.30 Menu. > MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....> >> >> If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus >> use it in dw_plat_pcie_ep_raise_irq also)? > I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct, > because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr, > but I also don't know where I can get the correct MSIX table. Sometime back when I tried raising MSI-X from EP, it was failing. It's quite possible dw_pcie_ep_raise_msix_irq function is not correct. MSI-X table can be obtained from the inbound ATU corresponding to the MSIX bar. IMO MSI-X support in EP mode needs rework. For instance set_msix should also take BAR number as input to be configured in the MSI-X capability. The function driver (pci-epf-test.c) should allocate memory taking into account the MSI-X table. Thanks Kishon
> -----Original Message----- > From: Andrew Murray <andrew.murray@arm.com> > Sent: 2019年8月16日 18:20 > To: Xiaowei Bao <xiaowei.bao@nxp.com> > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; > lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org; > M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; > linux-pci@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linuxppc-dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com> > Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of > MSI-X in EP mode > > On Fri, Aug 16, 2019 at 02:58:31AM +0000, Xiaowei Bao wrote: > > > > > > > -----Original Message----- > > > From: Andrew Murray <andrew.murray@arm.com> > > > Sent: 2019年8月15日 19:54 > > > To: Xiaowei Bao <xiaowei.bao@nxp.com> > > > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; > > > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; > > > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; > > > lorenzo.pieralisi@arm.com; arnd@arndb.de; > > > gregkh@linuxfoundation.org; M.h. Lian <minghuan.lian@nxp.com>; > > > Mingkai Hu <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>; > > > linux-pci@vger.kernel.org; devicetree@vger.kernel.org; > > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > > > linuxppc-dev@lists.ozlabs.org > > > Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode > > > of MSI-X in EP mode > > > > > > On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote: > > > > Add the doorbell mode of MSI-X in EP mode. > > > > > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> > > > > --- > > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 > ++++++++++++++ > > > > drivers/pci/controller/dwc/pcie-designware.h | 14 > ++++++++++++++ > > > > 2 files changed, 28 insertions(+) > > > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > index 75e2955..e3a7cdf 100644 > > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > > > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct > > > > dw_pcie_ep > > > *ep, u8 func_no, > > > > return 0; > > > > } > > > > > > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > > > func_no, > > > > + u16 interrupt_num) > > > > +{ > > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > > > > + u32 msg_data; > > > > + > > > > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | > > > > + (interrupt_num - 1); > > > > + > > > > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > > > u16 interrupt_num) > > > > > > Have I understood correctly that the hardware provides an > > > alternative mechanism that allows for raising MSI-X interrupts > > > without the bother of reading the capabilities registers? > > Yes, the hardware provide two way to MSI-X, please check the page 492 > > of > > DWC_pcie_dm_registers_4.30 Menu. > > MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell > > Register....> > > Thanks for the reference. > > > > > > > If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? > > > (And thus use it in dw_plat_pcie_ep_raise_irq also)? > > I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is > > not correct, because I think we can't get the MSIX table from the > > address ep->phys_base + tbl_addr, but I also don't know where I can get the > correct MSIX table. > > Well it looks like this function is used by snps,dw-pcie-ep and snps,dw-pcie, > perhaps the doorbell mode isn't available on that hardware. > > > > > > > > > > > { > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h > > > > b/drivers/pci/controller/dwc/pcie-designware.h > > > > index 2b291e8..cd903e9 100644 > > > > --- a/drivers/pci/controller/dwc/pcie-designware.h > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h > > > > @@ -88,6 +88,11 @@ > > > > #define PCIE_MISC_CONTROL_1_OFF 0x8BC > > > > #define PCIE_DBI_RO_WR_EN BIT(0) > > > > > > > > +#define PCIE_MSIX_DOORBELL 0x948 > > > > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24 > > > > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16 > > > > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15) > > > > > > The _VF defines are not used, I'd suggest removing them. > > In fact, I will add the SRIOV support in this file, the SRIOV feature > > have verified In my board, but I need wait the EP framework SRIOV > > patch merge, so I defined these two macros. > > I'd suggest adding the VF macros along with the SRIOV feature. OK, I will remove these two macros. Thanks. > > Thanks, > > Andrew Murray > > > > > > > Thanks, > > > > > > Andrew Murray > > > > > > > + > > > > /* > > > > * iATU Unroll-specific register definitions > > > > * From 4.80 core version the address translation will be made by > > > > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct > > > dw_pcie_ep *ep, u8 func_no, > > > > u8 interrupt_num); > > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > > > > u16 interrupt_num); > > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > > > func_no, > > > > + u16 interrupt_num); > > > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno > > > > bar); #else static inline void dw_pcie_ep_linkup(struct > > > > dw_pcie_ep *ep) @@ > > > > -431,6 +438,13 @@ static inline int > > > > dw_pcie_ep_raise_msix_irq(struct > > > dw_pcie_ep *ep, u8 func_no, > > > > return 0; > > > > } > > > > > > > > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct > > > > +dw_pcie_ep > > > *ep, > > > > + u8 func_no, > > > > + u16 interrupt_num) > > > > +{ > > > > + return 0; > > > > +} > > > > + > > > > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum > > > > pci_barno bar) { } > > > > -- > > > > 2.9.5 > > > > > > > > > > > > _______________________________________________ > > > > linux-arm-kernel mailing list > > > > linux-arm-kernel@lists.infradead.org > > > > http://lists > > > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=0 > 2 > > > > % > > > 7C0 > > > > > > > > 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972% > > > 7C686ea1d > > > > > > > > 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&sd > > > ata=dyrXB > > > > > > > > avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&reserved=0
> -----Original Message----- > From: Kishon Vijay Abraham I <kishon@ti.com> > Sent: 2019年8月16日 18:50 > To: Xiaowei Bao <xiaowei.bao@nxp.com>; Andrew Murray > <andrew.murray@arm.com> > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; > lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org; > M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@nxp.com>; > linux-pci@vger.kernel.org; devicetree@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linuxppc-dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com> > Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of > MSI-X in EP mode > > Hi, > > On 16/08/19 8:28 AM, Xiaowei Bao wrote: > > > > > >> -----Original Message----- > >> From: Andrew Murray <andrew.murray@arm.com> > >> Sent: 2019年8月15日 19:54 > >> To: Xiaowei Bao <xiaowei.bao@nxp.com> > >> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com; > >> bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com; > >> shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com; > >> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org; > >> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu > <mingkai.hu@nxp.com>; > >> Roy Zang <roy.zang@nxp.com>; linux-pci@vger.kernel.org; > >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > >> linux-arm-kernel@lists.infradead.org; linuxppc-dev@lists.ozlabs.org > >> Subject: Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode > >> of MSI-X in EP mode > >> > >> On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote: > >>> Add the doorbell mode of MSI-X in EP mode. > >>> > >>> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> > >>> --- > >>> drivers/pci/controller/dwc/pcie-designware-ep.c | 14 > ++++++++++++++ > >>> drivers/pci/controller/dwc/pcie-designware.h | 14 > ++++++++++++++ > >>> 2 files changed, 28 insertions(+) > >>> > >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c > >>> b/drivers/pci/controller/dwc/pcie-designware-ep.c > >>> index 75e2955..e3a7cdf 100644 > >>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > >>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > >>> @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct > dw_pcie_ep > >> *ep, u8 func_no, > >>> return 0; > >>> } > >>> > >>> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 > >> func_no, > >>> + u16 interrupt_num) > >>> +{ > >>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > >>> + u32 msg_data; > >>> + > >>> + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | > >>> + (interrupt_num - 1); > >>> + > >>> + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); > >>> + > >>> + return 0; > >>> +} > >>> + > >>> int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, > >>> u16 interrupt_num) > >> > >> Have I understood correctly that the hardware provides an alternative > >> mechanism that allows for raising MSI-X interrupts without the bother > >> of reading the capabilities registers? > > Yes, the hardware provide two way to MSI-X, please check the page 492 > > of > > DWC_pcie_dm_registers_4.30 Menu. > > MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell > > Register....> > >> > >> If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? > >> (And thus use it in dw_plat_pcie_ep_raise_irq also)? > > I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is > > not correct, because I think we can't get the MSIX table from the > > address ep->phys_base + tbl_addr, but I also don't know where I can get the > correct MSIX table. > > Sometime back when I tried raising MSI-X from EP, it was failing. It's quite > possible dw_pcie_ep_raise_msix_irq function is not correct. > > MSI-X table can be obtained from the inbound ATU corresponding to the MSIX > bar. > IMO MSI-X support in EP mode needs rework. For instance set_msix should > also take BAR number as input to be configured in the MSI-X capability. The > function driver (pci-epf-test.c) should allocate memory taking into account the > MSI-X table. Hi Kishon, Thanks a lot for your explain, yes, we can get the MSI-X table from the inbound ATU of the MSIX BAR. > > Thanks > Kishon
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 75e2955..e3a7cdf 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, return 0; } +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, + u16 interrupt_num) +{ + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); + u32 msg_data; + + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) | + (interrupt_num - 1); + + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data); + + return 0; +} + int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, u16 interrupt_num) { diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 2b291e8..cd903e9 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -88,6 +88,11 @@ #define PCIE_MISC_CONTROL_1_OFF 0x8BC #define PCIE_DBI_RO_WR_EN BIT(0) +#define PCIE_MSIX_DOORBELL 0x948 +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24 +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16 +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15) + /* * iATU Unroll-specific register definitions * From 4.80 core version the address translation will be made by unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no, u8 interrupt_num); int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, u16 interrupt_num); +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no, + u16 interrupt_num); void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar); #else static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@ -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no, return 0; } +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, + u8 func_no, + u16 interrupt_num) +{ + return 0; +} + static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar) { }
Add the doorbell mode of MSI-X in EP mode. Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com> --- drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++ drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++ 2 files changed, 28 insertions(+)