RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-16 Thread Xiaowei Bao 


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月16日 20:35
> To: Xiaowei Bao 
> Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> mark.rutl...@arm.com; shawn...@kernel.org; Leo Li
> ; kis...@ti.com; lorenzo.pieral...@arm.com;
> a...@arndb.de; gre...@linuxfoundation.org; M.h. Lian
> ; Roy Zang ;
> linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linuxppc-dev@lists.ozlabs.org; Z.q. Hou 
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
> 
> On Fri, Aug 16, 2019 at 11:00:01AM +, Xiaowei Bao wrote:
> >
> >
> > > -Original Message-
> > > From: Andrew Murray 
> > > Sent: 2019年8月16日 17:45
> > > To: Xiaowei Bao 
> > > Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> > > mark.rutl...@arm.com; shawn...@kernel.org; Leo Li
> > > ; kis...@ti.com; lorenzo.pieral...@arm.com;
> > > a...@arndb.de; gre...@linuxfoundation.org; M.h. Lian
> > > ; Roy Zang ;
> > > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > > linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > > linuxppc-dev@lists.ozlabs.org; Z.q. Hou 
> > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > support for DWC
> > >
> > > On Fri, Aug 16, 2019 at 02:55:41AM +, Xiaowei Bao wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Andrew Murray 
> > > > > Sent: 2019年8月15日 19:32
> > > > > To: Xiaowei Bao 
> > > > > Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> > > > > bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > > > > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > > > > lorenzo.pieral...@arm.com; a...@arndb.de;
> > > > > gre...@linuxfoundation.org; M.h. Lian ;
> > > > > Mingkai Hu ; Roy Zang ;
> > > > > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > > > > linux-ker...@vger.kernel.org;
> > > > > linux-arm-ker...@lists.infradead.org;
> > > > > linuxppc-dev@lists.ozlabs.org
> > > > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > > > support for DWC
> > > > >
> > > > > On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > > > > > Add multiple PFs support for DWC, different PF have different
> > > > > > config space, we use pf-offset property which get from the DTS
> > > > > > to access the different pF config space.
> > > > >
> > > > > Thanks for the patch. I haven't seen a cover letter for this
> > > > > series, is there one missing?
> > > > Maybe I miss, I will add you to review next time, thanks a lot for
> > > > your
> > > comments.
> > > > >
> > > > >
> > > > > >
> > > > > > Signed-off-by: Xiaowei Bao 
> > > > > > ---
> > > > > >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> > > > > +-
> > > > > >  drivers/pci/controller/dwc/pcie-designware.c| 105
> > > > > ++--
> > > > > >  drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
> > > > > >  include/linux/pci-epc.h |   1 +
> > > > > >  4 files changed, 164 insertions(+), 49 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > index 2bf5a35..75e2955 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep
> > > *ep)
> > > > > > pci_epc_linkup(epc);
> > > > > >  }
> > > > > >
> > > > > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > > > pci_barno
> > > > > bar,
> > > > > > -  int flags)
> > > > > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8
> func_no,
> > > > &g

Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-16 Thread Andrew Murray 
On Fri, Aug 16, 2019 at 11:00:01AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Andrew Murray 
> > Sent: 2019年8月16日 17:45
> > To: Xiaowei Bao 
> > Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> > mark.rutl...@arm.com; shawn...@kernel.org; Leo Li
> > ; kis...@ti.com; lorenzo.pieral...@arm.com;
> > a...@arndb.de; gre...@linuxfoundation.org; M.h. Lian
> > ; Roy Zang ;
> > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > linuxppc-dev@lists.ozlabs.org; Z.q. Hou 
> > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> > DWC
> > 
> > On Fri, Aug 16, 2019 at 02:55:41AM +, Xiaowei Bao wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Andrew Murray 
> > > > Sent: 2019年8月15日 19:32
> > > > To: Xiaowei Bao 
> > > > Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> > > > bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > > > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > > > lorenzo.pieral...@arm.com; a...@arndb.de;
> > > > gre...@linuxfoundation.org; M.h. Lian ;
> > > > Mingkai Hu ; Roy Zang ;
> > > > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > > > linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > > > linuxppc-dev@lists.ozlabs.org
> > > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > > support for DWC
> > > >
> > > > On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > > > > Add multiple PFs support for DWC, different PF have different
> > > > > config space, we use pf-offset property which get from the DTS to
> > > > > access the different pF config space.
> > > >
> > > > Thanks for the patch. I haven't seen a cover letter for this series,
> > > > is there one missing?
> > > Maybe I miss, I will add you to review next time, thanks a lot for your
> > comments.
> > > >
> > > >
> > > > >
> > > > > Signed-off-by: Xiaowei Bao 
> > > > > ---
> > > > >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> > > > +-
> > > > >  drivers/pci/controller/dwc/pcie-designware.c| 105
> > > > ++--
> > > > >  drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
> > > > >  include/linux/pci-epc.h |   1 +
> > > > >  4 files changed, 164 insertions(+), 49 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > index 2bf5a35..75e2955 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep
> > *ep)
> > > > >   pci_epc_linkup(epc);
> > > > >  }
> > > > >
> > > > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > > pci_barno
> > > > bar,
> > > > > -int flags)
> > > > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > > > > +enum pci_barno bar, int flags)
> > > > >  {
> > > > >   u32 reg;
> > > > > + struct pci_epc *epc = pci->ep.epc;
> > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > >
> > > > > - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > > + reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> > > >
> > > > I think I'd rather see this arithmetic (and the one for determining
> > > > pf_base) inside a macro or inline header function. This would make
> > > > this code more readable and reduce the chances of an error by avoiding
> > duplication of code.
> > > >
> > > > For example look at cdns_pcie_ep_fn_writeb and
> > > > ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that do
> > > > this.
> > > Agree, this looks fine, thanks a lot for your comments,

RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-16 Thread Xiaowei Bao 


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月16日 17:45
> To: Xiaowei Bao 
> Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> mark.rutl...@arm.com; shawn...@kernel.org; Leo Li
> ; kis...@ti.com; lorenzo.pieral...@arm.com;
> a...@arndb.de; gre...@linuxfoundation.org; M.h. Lian
> ; Roy Zang ;
> linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> linuxppc-dev@lists.ozlabs.org; Z.q. Hou 
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
> 
> On Fri, Aug 16, 2019 at 02:55:41AM +, Xiaowei Bao wrote:
> >
> >
> > > -Original Message-
> > > From: Andrew Murray 
> > > Sent: 2019年8月15日 19:32
> > > To: Xiaowei Bao 
> > > Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> > > bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > > lorenzo.pieral...@arm.com; a...@arndb.de;
> > > gre...@linuxfoundation.org; M.h. Lian ;
> > > Mingkai Hu ; Roy Zang ;
> > > linux-...@vger.kernel.org; devicet...@vger.kernel.org;
> > > linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > > linuxppc-dev@lists.ozlabs.org
> > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > support for DWC
> > >
> > > On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > > > Add multiple PFs support for DWC, different PF have different
> > > > config space, we use pf-offset property which get from the DTS to
> > > > access the different pF config space.
> > >
> > > Thanks for the patch. I haven't seen a cover letter for this series,
> > > is there one missing?
> > Maybe I miss, I will add you to review next time, thanks a lot for your
> comments.
> > >
> > >
> > > >
> > > > Signed-off-by: Xiaowei Bao 
> > > > ---
> > > >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> > > +-
> > > >  drivers/pci/controller/dwc/pcie-designware.c| 105
> > > ++--
> > > >  drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
> > > >  include/linux/pci-epc.h |   1 +
> > > >  4 files changed, 164 insertions(+), 49 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > index 2bf5a35..75e2955 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep
> *ep)
> > > > pci_epc_linkup(epc);
> > > >  }
> > > >
> > > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > pci_barno
> > > bar,
> > > > -  int flags)
> > > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > > > +  enum pci_barno bar, int flags)
> > > >  {
> > > > u32 reg;
> > > > +   struct pci_epc *epc = pci->ep.epc;
> > > > +   u32 pf_base = func_no * epc->pf_offset;
> > > >
> > > > -   reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > +   reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> > >
> > > I think I'd rather see this arithmetic (and the one for determining
> > > pf_base) inside a macro or inline header function. This would make
> > > this code more readable and reduce the chances of an error by avoiding
> duplication of code.
> > >
> > > For example look at cdns_pcie_ep_fn_writeb and
> > > ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that do
> > > this.
> > Agree, this looks fine, thanks a lot for your comments, I will use
> > this way to access the registers in next version patch.
> > >
> > >
> > > > dw_pcie_dbi_ro_wr_en(pci);
> > > > dw_pcie_writel_dbi2(pci, reg, 0x0);
> > > > dw_pcie_writel_dbi(pci, reg, 0x0); @@ -37,7 +39,12 @@ static
> > > > void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> > > > bar,
> > > >
> > > >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enu

Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-16 Thread Andrew Murray 
On Fri, Aug 16, 2019 at 02:55:41AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Andrew Murray 
> > Sent: 2019年8月15日 19:32
> > To: Xiaowei Bao 
> > Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> > bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > lorenzo.pieral...@arm.com; a...@arndb.de; gre...@linuxfoundation.org;
> > M.h. Lian ; Mingkai Hu ;
> > Roy Zang ; linux-...@vger.kernel.org;
> > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org;
> > linux-arm-ker...@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> > DWC
> > 
> > On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > > Add multiple PFs support for DWC, different PF have different config
> > > space, we use pf-offset property which get from the DTS to access the
> > > different pF config space.
> > 
> > Thanks for the patch. I haven't seen a cover letter for this series, is 
> > there one
> > missing?
> Maybe I miss, I will add you to review next time, thanks a lot for your 
> comments.
> > 
> > 
> > >
> > > Signed-off-by: Xiaowei Bao 
> > > ---
> > >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> > +-
> > >  drivers/pci/controller/dwc/pcie-designware.c| 105
> > ++--
> > >  drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
> > >  include/linux/pci-epc.h |   1 +
> > >  4 files changed, 164 insertions(+), 49 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 2bf5a35..75e2955 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > >   pci_epc_linkup(epc);
> > >  }
> > >
> > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> > bar,
> > > -int flags)
> > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > > +enum pci_barno bar, int flags)
> > >  {
> > >   u32 reg;
> > > + struct pci_epc *epc = pci->ep.epc;
> > > + u32 pf_base = func_no * epc->pf_offset;
> > >
> > > - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > + reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> > 
> > I think I'd rather see this arithmetic (and the one for determining pf_base)
> > inside a macro or inline header function. This would make this code more
> > readable and reduce the chances of an error by avoiding duplication of code.
> > 
> > For example look at cdns_pcie_ep_fn_writeb and
> > ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that do
> > this.
> Agree, this looks fine, thanks a lot for your comments, I will use this way 
> to access
> the registers in next version patch.
> > 
> > 
> > >   dw_pcie_dbi_ro_wr_en(pci);
> > >   dw_pcie_writel_dbi2(pci, reg, 0x0);
> > >   dw_pcie_writel_dbi(pci, reg, 0x0);
> > > @@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie
> > > *pci, enum pci_barno bar,
> > >
> > >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)  {
> > > - __dw_pcie_ep_reset_bar(pci, bar, 0);
> > > + u8 func_no, funcs;
> > > +
> > > + funcs = pci->ep.epc->max_functions;
> > > +
> > > + for (func_no = 0; func_no < funcs; func_no++)
> > > + __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> > >  }
> > >
> > >  static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> > > @@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc
> > > *epc, u8 func_no,  {
> > >   struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > >   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + u32 pf_base = func_no * epc->pf_offset;
> > >
> > >   dw_pcie_dbi_ro_wr_en(pci);
> > > - dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> > > - dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> > > - dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> > > - dw_pcie_writeb_dbi(pci, PCI_C

RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-15 Thread Xiaowei Bao 


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月15日 19:32
> To: Xiaowei Bao 
> Cc: jingooh...@gmail.com; gustavo.pimen...@synopsys.com;
> bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> lorenzo.pieral...@arm.com; a...@arndb.de; gre...@linuxfoundation.org;
> M.h. Lian ; Mingkai Hu ;
> Roy Zang ; linux-...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-ker...@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
> 
> On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > Add multiple PFs support for DWC, different PF have different config
> > space, we use pf-offset property which get from the DTS to access the
> > different pF config space.
> 
> Thanks for the patch. I haven't seen a cover letter for this series, is there 
> one
> missing?
Maybe I miss, I will add you to review next time, thanks a lot for your 
comments.
> 
> 
> >
> > Signed-off-by: Xiaowei Bao 
> > ---
> >  drivers/pci/controller/dwc/pcie-designware-ep.c |  97
> +-
> >  drivers/pci/controller/dwc/pcie-designware.c| 105
> ++--
> >  drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
> >  include/linux/pci-epc.h |   1 +
> >  4 files changed, 164 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 2bf5a35..75e2955 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> > pci_epc_linkup(epc);
> >  }
> >
> > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> bar,
> > -  int flags)
> > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > +  enum pci_barno bar, int flags)
> >  {
> > u32 reg;
> > +   struct pci_epc *epc = pci->ep.epc;
> > +   u32 pf_base = func_no * epc->pf_offset;
> >
> > -   reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > +   reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> 
> I think I'd rather see this arithmetic (and the one for determining pf_base)
> inside a macro or inline header function. This would make this code more
> readable and reduce the chances of an error by avoiding duplication of code.
> 
> For example look at cdns_pcie_ep_fn_writeb and
> ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that do
> this.
Agree, this looks fine, thanks a lot for your comments, I will use this way to 
access
the registers in next version patch.
> 
> 
> > dw_pcie_dbi_ro_wr_en(pci);
> > dw_pcie_writel_dbi2(pci, reg, 0x0);
> > dw_pcie_writel_dbi(pci, reg, 0x0);
> > @@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie
> > *pci, enum pci_barno bar,
> >
> >  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)  {
> > -   __dw_pcie_ep_reset_bar(pci, bar, 0);
> > +   u8 func_no, funcs;
> > +
> > +   funcs = pci->ep.epc->max_functions;
> > +
> > +   for (func_no = 0; func_no < funcs; func_no++)
> > +   __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> >  }
> >
> >  static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> > @@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc
> > *epc, u8 func_no,  {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +   u32 pf_base = func_no * epc->pf_offset;
> >
> > dw_pcie_dbi_ro_wr_en(pci);
> > -   dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> > -   dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> > -   dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> > -   dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> > -   dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> > +   dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID, hdr->vendorid);
> > +   dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID, hdr->deviceid);
> > +   dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID, hdr->revid);
> > +   dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG, hdr->progif_code);
> > +   dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
> >

Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-15 Thread Andrew Murray 
On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> Add multiple PFs support for DWC, different PF have different config space,
> we use pf-offset property which get from the DTS to access the different pF
> config space.

Thanks for the patch. I haven't seen a cover letter for this series, is there
one missing?


> 
> Signed-off-by: Xiaowei Bao 
> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c |  97 +-
>  drivers/pci/controller/dwc/pcie-designware.c| 105 
> ++--
>  drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
>  include/linux/pci-epc.h |   1 +
>  4 files changed, 164 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c 
> b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2bf5a35..75e2955 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>   pci_epc_linkup(epc);
>  }
>  
> -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar,
> -int flags)
> +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> +enum pci_barno bar, int flags)
>  {
>   u32 reg;
> + struct pci_epc *epc = pci->ep.epc;
> + u32 pf_base = func_no * epc->pf_offset;
>  
> - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> + reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);

I think I'd rather see this arithmetic (and the one for determining pf_base)
inside a macro or inline header function. This would make this code more 
readable
and reduce the chances of an error by avoiding duplication of code.

For example look at cdns_pcie_ep_fn_writeb and ROCKCHIP_PCIE_EP_FUNC_BASE for
examples of other EP drivers that do this.


>   dw_pcie_dbi_ro_wr_en(pci);
>   dw_pcie_writel_dbi2(pci, reg, 0x0);
>   dw_pcie_writel_dbi(pci, reg, 0x0);
> @@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, 
> enum pci_barno bar,
>  
>  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
> - __dw_pcie_ep_reset_bar(pci, bar, 0);
> + u8 func_no, funcs;
> +
> + funcs = pci->ep.epc->max_functions;
> +
> + for (func_no = 0; func_no < funcs; func_no++)
> + __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
>  }
>  
>  static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> @@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, 
> u8 func_no,
>  {
>   struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + u32 pf_base = func_no * epc->pf_offset;
>  
>   dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> - dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> - dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> - dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> - dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> + dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID, hdr->vendorid);
> + dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID, hdr->deviceid);
> + dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID, hdr->revid);
> + dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG, hdr->progif_code);
> + dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
>  hdr->subclass_code | hdr->baseclass_code << 8);
> - dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
> + dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
>  hdr->cache_line_size);
> - dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
> + dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_VENDOR_ID,
>  hdr->subsys_vendor_id);
> - dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> - dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
> + dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID, hdr->subsys_id);
> + dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
>  hdr->interrupt_pin);
>   dw_pcie_dbi_ro_wr_dis(pci);
>  
>   return 0;
>  }
>  
> -static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar,
> -   dma_addr_t cpu_addr,
> +static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> +   enum pci_barno bar, dma_addr_t cpu_addr,
> enum dw_pcie_as_type as_type)
>  {
>   int ret;
> @@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, 
> enum pci_barno bar,
>   return -EINVAL;
>   }
>  
> - ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
> + ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar, cpu_addr,
>  as_type);
>   if

[PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC

2019-08-15 Thread Xiaowei Bao 
Add multiple PFs support for DWC, different PF have different config space,
we use pf-offset property which get from the DTS to access the different pF
config space.

Signed-off-by: Xiaowei Bao 
---
 drivers/pci/controller/dwc/pcie-designware-ep.c |  97 +-
 drivers/pci/controller/dwc/pcie-designware.c| 105 ++--
 drivers/pci/controller/dwc/pcie-designware.h|  10 ++-
 include/linux/pci-epc.h |   1 +
 4 files changed, 164 insertions(+), 49 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c 
b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 2bf5a35..75e2955 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
pci_epc_linkup(epc);
 }
 
-static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar,
-  int flags)
+static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
+  enum pci_barno bar, int flags)
 {
u32 reg;
+   struct pci_epc *epc = pci->ep.epc;
+   u32 pf_base = func_no * epc->pf_offset;
 
-   reg = PCI_BASE_ADDRESS_0 + (4 * bar);
+   reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
dw_pcie_dbi_ro_wr_en(pci);
dw_pcie_writel_dbi2(pci, reg, 0x0);
dw_pcie_writel_dbi(pci, reg, 0x0);
@@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum 
pci_barno bar,
 
 void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
 {
-   __dw_pcie_ep_reset_bar(pci, bar, 0);
+   u8 func_no, funcs;
+
+   funcs = pci->ep.epc->max_functions;
+
+   for (func_no = 0; func_no < funcs; func_no++)
+   __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
 }
 
 static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
@@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 
func_no,
 {
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+   u32 pf_base = func_no * epc->pf_offset;
 
dw_pcie_dbi_ro_wr_en(pci);
-   dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
-   dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
-   dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
-   dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
-   dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
+   dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID, hdr->vendorid);
+   dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID, hdr->deviceid);
+   dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID, hdr->revid);
+   dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG, hdr->progif_code);
+   dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
   hdr->subclass_code | hdr->baseclass_code << 8);
-   dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
+   dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
   hdr->cache_line_size);
-   dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
+   dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_VENDOR_ID,
   hdr->subsys_vendor_id);
-   dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID, hdr->subsys_id);
-   dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
+   dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID, hdr->subsys_id);
+   dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
   hdr->interrupt_pin);
dw_pcie_dbi_ro_wr_dis(pci);
 
return 0;
 }
 
-static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar,
- dma_addr_t cpu_addr,
+static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
+ enum pci_barno bar, dma_addr_t cpu_addr,
  enum dw_pcie_as_type as_type)
 {
int ret;
@@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, 
enum pci_barno bar,
return -EINVAL;
}
 
-   ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
+   ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar, cpu_addr,
   as_type);
if (ret < 0) {
dev_err(pci->dev, "Failed to program IB window\n");
@@ -125,7 +133,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, 
enum pci_barno bar,
return 0;
 }
 
-static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t 
phys_addr,
+static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
+  phys_addr_t phys_addr,
   u64 pci_addr, size_t size)
 {
u32 free_win;
@@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, 
phys_addr_t phys_addr,