RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-28 Thread Xiaowei Bao


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月28日 17:01
> To: Xiaowei Bao 
> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
> Lian ; Mingkai Hu ; Roy
> Zang ; jingooh...@gmail.com;
> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
> 
> On Wed, Aug 28, 2019 at 04:29:32AM +, Xiaowei Bao wrote:
> >
> >
> > > -Original Message-
> > > From: Andrew Murray 
> > > Sent: 2019年8月27日 21:34
> > > To: Xiaowei Bao 
> > > Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > > lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org;
> M.h.
> > > Lian ; Mingkai Hu ; Roy
> > > Zang ; jingooh...@gmail.com;
> > > gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support
> > > for ls1088a and ls2088a
> > >
> > > On Mon, Aug 26, 2019 at 09:49:35AM +, Xiaowei Bao wrote:
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Andrew Murray 
> > > > > Sent: 2019年8月23日 22:28
> > > > > To: Xiaowei Bao 
> > > > > Cc: bhelg...@google.com; robh...@kernel.org;
> > > > > mark.rutl...@arm.com; shawn...@kernel.org; Leo Li
> > > > > ; kis...@ti.com; lorenzo.pieral...@arm.co;
> > > > > a...@arndb.de; gre...@linuxfoundation.org;
> > > M.h.
> > > > > Lian ; Mingkai Hu ;
> > > > > Roy Zang ; jingooh...@gmail.com;
> > > > > gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode
> > > > > support for ls1088a and ls2088a
> > > > >
> > > > > On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > > > > > Add PCIe EP mode support for ls1088a and ls2088a, there are
> > > > > > some difference between LS1 and LS2 platform, so refactor the
> > > > > > code of the EP driver.
> > > > > >
> > > > > > Signed-off-by: Xiaowei Bao 
> > > > > > ---
> > > > > > v2:
> > > > > >  - New mechanism for layerscape EP driver.
> > > > >
> > > > > Was there a v1 of this patch?
> > > > >
> > > > > >
> > > > > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > > > > > --
> > > > > >  1 file changed, 58 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > index 7ca5fe8..2a66f07 100644
> > > > > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > > @@ -20,27 +20,29 @@
> > > > > >
> > > > > >  #define PCIE_DBI2_OFFSET   0x1000  /* DBI2 base address*/
> > > > > >
> > > > > > -struct ls_pcie_ep {
> > > > > > -   struct dw_pcie  *pci;
> > > > > > -   struct pci_epc_features *ls_epc;
> > > > > > +#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > > > > > +
> > > > > > +struct ls_pcie_ep_drvdata {
> > > > > > +   u32 func_offset;
> > > > > > +   const struct dw_pcie_ep_ops *ops;
> > > > > > +   const struct dw_pcie_ops*dw_pcie_ops;
> > > > > >  };
> > > > > >
> > > > > > -#define to_ls_

Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-28 Thread Andrew Murray
On Wed, Aug 28, 2019 at 04:29:32AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Andrew Murray 
> > Sent: 2019年8月27日 21:34
> > To: Xiaowei Bao 
> > Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
> > Lian ; Mingkai Hu ; Roy
> > Zang ; jingooh...@gmail.com;
> > gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
> > ls1088a and ls2088a
> > 
> > On Mon, Aug 26, 2019 at 09:49:35AM +, Xiaowei Bao wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Andrew Murray 
> > > > Sent: 2019年8月23日 22:28
> > > > To: Xiaowei Bao 
> > > > Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > > > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > > > lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org;
> > M.h.
> > > > Lian ; Mingkai Hu ; Roy
> > > > Zang ; jingooh...@gmail.com;
> > > > gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support
> > > > for ls1088a and ls2088a
> > > >
> > > > On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > > > > Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > > > > difference between LS1 and LS2 platform, so refactor the code of
> > > > > the EP driver.
> > > > >
> > > > > Signed-off-by: Xiaowei Bao 
> > > > > ---
> > > > > v2:
> > > > >  - New mechanism for layerscape EP driver.
> > > >
> > > > Was there a v1 of this patch?
> > > >
> > > > >
> > > > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > > > > --
> > > > >  1 file changed, 58 insertions(+), 18 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > index 7ca5fe8..2a66f07 100644
> > > > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > > @@ -20,27 +20,29 @@
> > > > >
> > > > >  #define PCIE_DBI2_OFFSET 0x1000  /* DBI2 base address*/
> > > > >
> > > > > -struct ls_pcie_ep {
> > > > > - struct dw_pcie  *pci;
> > > > > - struct pci_epc_features *ls_epc;
> > > > > +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > > +
> > > > > +struct ls_pcie_ep_drvdata {
> > > > > + u32 func_offset;
> > > > > + const struct dw_pcie_ep_ops *ops;
> > > > > + const struct dw_pcie_ops*dw_pcie_ops;
> > > > >  };
> > > > >
> > > > > -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > > +struct ls_pcie_ep {
> > > > > + struct dw_pcie  *pci;
> > > > > + struct pci_epc_features *ls_epc;
> > > > > + const struct ls_pcie_ep_drvdata *drvdata; };
> > > > >
> > > > >  static int ls_pcie_establish_link(struct dw_pcie *pci)  {
> > > > >   return 0;
> > > > >  }
> > > > >
> > > > > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > > > > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > > > >   .start_link = ls_pcie_establish_link,  };
> > > > >
> > > > > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > > > - { .compatible = "fsl,ls-pcie-ep",},
> > > > > - { },
> > > > > -};
> > > > > -
> > > > >  static c

RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-27 Thread Xiaowei Bao


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月27日 21:34
> To: Xiaowei Bao 
> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
> Lian ; Mingkai Hu ; Roy
> Zang ; jingooh...@gmail.com;
> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
> 
> On Mon, Aug 26, 2019 at 09:49:35AM +, Xiaowei Bao wrote:
> >
> >
> > > -Original Message-
> > > From: Andrew Murray 
> > > Sent: 2019年8月23日 22:28
> > > To: Xiaowei Bao 
> > > Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > > lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org;
> M.h.
> > > Lian ; Mingkai Hu ; Roy
> > > Zang ; jingooh...@gmail.com;
> > > gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support
> > > for ls1088a and ls2088a
> > >
> > > On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > > > Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > > > difference between LS1 and LS2 platform, so refactor the code of
> > > > the EP driver.
> > > >
> > > > Signed-off-by: Xiaowei Bao 
> > > > ---
> > > > v2:
> > > >  - New mechanism for layerscape EP driver.
> > >
> > > Was there a v1 of this patch?
> > >
> > > >
> > > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > > > --
> > > >  1 file changed, 58 insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > index 7ca5fe8..2a66f07 100644
> > > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > @@ -20,27 +20,29 @@
> > > >
> > > >  #define PCIE_DBI2_OFFSET   0x1000  /* DBI2 base address*/
> > > >
> > > > -struct ls_pcie_ep {
> > > > -   struct dw_pcie  *pci;
> > > > -   struct pci_epc_features *ls_epc;
> > > > +#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > > > +
> > > > +struct ls_pcie_ep_drvdata {
> > > > +   u32 func_offset;
> > > > +   const struct dw_pcie_ep_ops *ops;
> > > > +   const struct dw_pcie_ops*dw_pcie_ops;
> > > >  };
> > > >
> > > > -#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > > > +struct ls_pcie_ep {
> > > > +   struct dw_pcie  *pci;
> > > > +   struct pci_epc_features *ls_epc;
> > > > +   const struct ls_pcie_ep_drvdata *drvdata; };
> > > >
> > > >  static int ls_pcie_establish_link(struct dw_pcie *pci)  {
> > > > return 0;
> > > >  }
> > > >
> > > > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > > > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > > > .start_link = ls_pcie_establish_link,  };
> > > >
> > > > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > > -   { .compatible = "fsl,ls-pcie-ep",},
> > > > -   { },
> > > > -};
> > > > -
> > > >  static const struct pci_epc_features*
> > > > ls_pcie_ep_get_features(struct dw_pcie_ep *ep)  { @@ -82,10 +84,44
> > > > @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > > }
> > > >  }
> > > >
> > > > -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep
> *ep,
> > > > +   u8 func_no)
> 

RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-27 Thread Xiaowei Bao


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月27日 22:49
> To: Xiaowei Bao 
> Cc: christophe leroy ; mark.rutl...@arm.com; Roy
> Zang ; lorenzo.pieral...@arm.co; a...@arndb.de;
> devicet...@vger.kernel.org; gre...@linuxfoundation.org;
> linuxppc-dev@lists.ozlabs.org; linux-...@vger.kernel.org;
> linux-ker...@vger.kernel.org; kis...@ti.com; M.h. Lian
> ; robh...@kernel.org;
> gustavo.pimen...@synopsys.com; jingooh...@gmail.com;
> bhelg...@google.com; Leo Li ; shawn...@kernel.org;
> Mingkai Hu ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
> 
> On Sun, Aug 25, 2019 at 03:07:32AM +, Xiaowei Bao wrote:
> >
> >
> > > -Original Message-
> > > From: christophe leroy 
> > > Sent: 2019年8月24日 14:45
> > > To: Xiaowei Bao ; Andrew Murray
> > > 
> > > Cc: mark.rutl...@arm.com; Roy Zang ;
> > > lorenzo.pieral...@arm.co; a...@arndb.de; devicet...@vger.kernel.org;
> > > gre...@linuxfoundation.org; linuxppc-dev@lists.ozlabs.org;
> > > linux-...@vger.kernel.org; linux-ker...@vger.kernel.org; kis...@ti.com;
> M.h.
> > > Lian ; robh...@kernel.org;
> > > gustavo.pimen...@synopsys.com; jingooh...@gmail.com;
> > > bhelg...@google.com; Leo Li ;
> > > shawn...@kernel.org; Mingkai Hu ;
> > > linux-arm-ker...@lists.infradead.org
> > > Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support
> > > for ls1088a and ls2088a
> > >
> > >
> > >
> > > Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :
> > > >
> > > >
> > > >> -Original Message-
> > > >> From: Andrew Murray 
> > > >> Sent: 2019年8月23日 22:28
> > > >> To: Xiaowei Bao 
> > > >> Cc: bhelg...@google.com; robh...@kernel.org;
> > > >> mark.rutl...@arm.com; shawn...@kernel.org; Leo Li
> > > >> ; kis...@ti.com; lorenzo.pieral...@arm.co;
> > > >> a...@arndb.de; gre...@linuxfoundation.org;
> > > M.h.
> > > >> Lian ; Mingkai Hu ;
> > > >> Roy Zang ; jingooh...@gmail.com;
> > > >> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode
> > > >> support for ls1088a and ls2088a
> > > >>
> > > >> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > > >>> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > > >>> difference between LS1 and LS2 platform, so refactor the code of
> > > >>> the EP driver.
> > > >>>
> > > >>> Signed-off-by: Xiaowei Bao 
> > > >>> ---
> > > >>> v2:
> > > >>>   - New mechanism for layerscape EP driver.
> > > >>
> > > >> Was there a v1 of this patch?
> > > >
> > > > Yes, but I don't know how to comments, ^_^
> > >
> > > As far as I can see, in the previous version of the series
> > > (https://patch
> > >
> work.ozlabs.org%2Fproject%2Flinuxppc-dev%2Flist%2F%3Fseries%3D125315
> > > %26state%3D*data=02%7C01%7Cxiaowei.bao%40nxp.com%7C1b
> efe9
> > >
> a67c8046f9535e08d7285eaab6%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> > >
> 7C0%7C0%7C637022259387139020sdata=p4wbycd04Z7qRUfAoZtwc
> > > UP7pR%2FuA3%2FjVcWMz6YyQVQ%3Dreserved=0),
> > > the 8/10 was something completely different, and I can't find any
> > > other patch in the series that could have been the v1 of this patch.
> >
> > Thanks, I will correct it to v1 in next version patch.
> 
> I think you numbered it correctly (so please leave it as v2, referring to the
> patch series revision) - I got confused trying to find a previous version of 
> this
> patch.
> 
> Perhaps in the future when new patches are introduced in a series you can
> indicate that in the description patch revision history (e.g. introduced in 
> v2).

OK, thanks for your help, I will update it in the next version patch.

Thanks 
Xiaowei

> 
> Thanks,
> 
> Andrew Murray
> 
> >
> > >
> > > Christophe
> > >
> > > >
> > > >>
> > > >>>
> > > >>>   drivers/pci/controller/dwc/pci-layerscape-ep.c | 

Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-27 Thread Andrew Murray
On Sun, Aug 25, 2019 at 03:07:32AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: christophe leroy 
> > Sent: 2019年8月24日 14:45
> > To: Xiaowei Bao ; Andrew Murray
> > 
> > Cc: mark.rutl...@arm.com; Roy Zang ;
> > lorenzo.pieral...@arm.co; a...@arndb.de; devicet...@vger.kernel.org;
> > gre...@linuxfoundation.org; linuxppc-dev@lists.ozlabs.org;
> > linux-...@vger.kernel.org; linux-ker...@vger.kernel.org; kis...@ti.com; M.h.
> > Lian ; robh...@kernel.org;
> > gustavo.pimen...@synopsys.com; jingooh...@gmail.com;
> > bhelg...@google.com; Leo Li ; shawn...@kernel.org;
> > Mingkai Hu ; linux-arm-ker...@lists.infradead.org
> > Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
> > ls1088a and ls2088a
> > 
> > 
> > 
> > Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :
> > >
> > >
> > >> -Original Message-
> > >> From: Andrew Murray 
> > >> Sent: 2019年8月23日 22:28
> > >> To: Xiaowei Bao 
> > >> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > >> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > >> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org;
> > M.h.
> > >> Lian ; Mingkai Hu ; Roy
> > >> Zang ; jingooh...@gmail.com;
> > >> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support
> > >> for ls1088a and ls2088a
> > >>
> > >> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > >>> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > >>> difference between LS1 and LS2 platform, so refactor the code of the
> > >>> EP driver.
> > >>>
> > >>> Signed-off-by: Xiaowei Bao 
> > >>> ---
> > >>> v2:
> > >>>   - New mechanism for layerscape EP driver.
> > >>
> > >> Was there a v1 of this patch?
> > >
> > > Yes, but I don't know how to comments, ^_^
> > 
> > As far as I can see, in the previous version of the series
> > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> > work.ozlabs.org%2Fproject%2Flinuxppc-dev%2Flist%2F%3Fseries%3D125315
> > %26state%3D*data=02%7C01%7Cxiaowei.bao%40nxp.com%7C1befe9
> > a67c8046f9535e08d7285eaab6%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> > 7C0%7C0%7C637022259387139020sdata=p4wbycd04Z7qRUfAoZtwc
> > UP7pR%2FuA3%2FjVcWMz6YyQVQ%3Dreserved=0),
> > the 8/10 was something completely different, and I can't find any other 
> > patch
> > in the series that could have been the v1 of this patch.
> 
> Thanks, I will correct it to v1 in next version patch.

I think you numbered it correctly (so please leave it as v2, referring to
the patch series revision) - I got confused trying to find a previous
version of this patch.

Perhaps in the future when new patches are introduced in a series you can
indicate that in the description patch revision history (e.g. introduced
in v2).

Thanks,

Andrew Murray 

> 
> > 
> > Christophe
> > 
> > >
> > >>
> > >>>
> > >>>   drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > >>> --
> > >>>   1 file changed, 58 insertions(+), 18 deletions(-)
> > >>>
> > >>> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > >>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > >>> index 7ca5fe8..2a66f07 100644
> > >>> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > >>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > >>> @@ -20,27 +20,29 @@
> > >>>
> > >>>   #define PCIE_DBI2_OFFSET  0x1000  /* DBI2 base address*/
> > >>>
> > >>> -struct ls_pcie_ep {
> > >>> -   struct dw_pcie  *pci;
> > >>> -   struct pci_epc_features *ls_epc;
> > >>> +#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > >>> +
> > >>> +struct ls_pcie_ep_drvdata {
> > >>> +   u32 func_offset;
> > >>> +   const struct dw_pcie_ep_ops *ops;
> > >>> +   const struct dw_pcie_ops*dw_pci

Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-27 Thread Andrew Murray
On Mon, Aug 26, 2019 at 09:49:35AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Andrew Murray 
> > Sent: 2019年8月23日 22:28
> > To: Xiaowei Bao 
> > Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > shawn...@kernel.org; Leo Li ; kis...@ti.com;
> > lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
> > Lian ; Mingkai Hu ; Roy
> > Zang ; jingooh...@gmail.com;
> > gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
> > ls1088a and ls2088a
> > 
> > On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > > Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > > difference between LS1 and LS2 platform, so refactor the code of the
> > > EP driver.
> > >
> > > Signed-off-by: Xiaowei Bao 
> > > ---
> > > v2:
> > >  - New mechanism for layerscape EP driver.
> > 
> > Was there a v1 of this patch?
> > 
> > >
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > > --
> > >  1 file changed, 58 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index 7ca5fe8..2a66f07 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -20,27 +20,29 @@
> > >
> > >  #define PCIE_DBI2_OFFSET 0x1000  /* DBI2 base address*/
> > >
> > > -struct ls_pcie_ep {
> > > - struct dw_pcie  *pci;
> > > - struct pci_epc_features *ls_epc;
> > > +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > +
> > > +struct ls_pcie_ep_drvdata {
> > > + u32 func_offset;
> > > + const struct dw_pcie_ep_ops *ops;
> > > + const struct dw_pcie_ops*dw_pcie_ops;
> > >  };
> > >
> > > -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > +struct ls_pcie_ep {
> > > + struct dw_pcie  *pci;
> > > + struct pci_epc_features *ls_epc;
> > > + const struct ls_pcie_ep_drvdata *drvdata; };
> > >
> > >  static int ls_pcie_establish_link(struct dw_pcie *pci)  {
> > >   return 0;
> > >  }
> > >
> > > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > >   .start_link = ls_pcie_establish_link,  };
> > >
> > > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > > - { .compatible = "fsl,ls-pcie-ep",},
> > > - { },
> > > -};
> > > -
> > >  static const struct pci_epc_features*  ls_pcie_ep_get_features(struct
> > > dw_pcie_ep *ep)  { @@ -82,10 +84,44 @@ static int
> > > ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > >   }
> > >  }
> > >
> > > -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> > > + u8 func_no)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > + u8 header_type;
> > > +
> > > + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> > > +
> > > + if (header_type & (1 << 7))
> > > + return pcie->drvdata->func_offset * func_no;
> > > + else
> > > + return 0;
> > 
> > It looks like there isn't a PCI define for multi function, the nearest I 
> > could find
> > was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
> > above the test might be helpful to explain the test.
> 
> OK, I will add a comment above this code.
> 
> > 
> > As the ls_pcie_ep_drvdata structures are static, the unset .func_offset 
> > will be
> > initialised to 0, so you could just drop the test above.
> 
> Due to the different PCIe controller have different property, e.g. PCIe 
> controller1 support
> multiple function feature, but PCIe controller2 don't support this feature, 
> so I need to check
> which controller 

RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-26 Thread Xiaowei Bao


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月23日 22:28
> To: Xiaowei Bao 
> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
> Lian ; Mingkai Hu ; Roy
> Zang ; jingooh...@gmail.com;
> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
> 
> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > difference between LS1 and LS2 platform, so refactor the code of the
> > EP driver.
> >
> > Signed-off-by: Xiaowei Bao 
> > ---
> > v2:
> >  - New mechanism for layerscape EP driver.
> 
> Was there a v1 of this patch?
> 
> >
> >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > --
> >  1 file changed, 58 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index 7ca5fe8..2a66f07 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -20,27 +20,29 @@
> >
> >  #define PCIE_DBI2_OFFSET   0x1000  /* DBI2 base address*/
> >
> > -struct ls_pcie_ep {
> > -   struct dw_pcie  *pci;
> > -   struct pci_epc_features *ls_epc;
> > +#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > +
> > +struct ls_pcie_ep_drvdata {
> > +   u32 func_offset;
> > +   const struct dw_pcie_ep_ops *ops;
> > +   const struct dw_pcie_ops*dw_pcie_ops;
> >  };
> >
> > -#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > +struct ls_pcie_ep {
> > +   struct dw_pcie  *pci;
> > +   struct pci_epc_features *ls_epc;
> > +   const struct ls_pcie_ep_drvdata *drvdata; };
> >
> >  static int ls_pcie_establish_link(struct dw_pcie *pci)  {
> > return 0;
> >  }
> >
> > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > .start_link = ls_pcie_establish_link,  };
> >
> > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > -   { .compatible = "fsl,ls-pcie-ep",},
> > -   { },
> > -};
> > -
> >  static const struct pci_epc_features*  ls_pcie_ep_get_features(struct
> > dw_pcie_ep *ep)  { @@ -82,10 +84,44 @@ static int
> > ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > }
> >  }
> >
> > -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> > +   u8 func_no)
> > +{
> > +   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +   struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > +   u8 header_type;
> > +
> > +   header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> > +
> > +   if (header_type & (1 << 7))
> > +   return pcie->drvdata->func_offset * func_no;
> > +   else
> > +   return 0;
> 
> It looks like there isn't a PCI define for multi function, the nearest I 
> could find
> was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
> above the test might be helpful to explain the test.

OK, I will add a comment above this code.

> 
> As the ls_pcie_ep_drvdata structures are static, the unset .func_offset will 
> be
> initialised to 0, so you could just drop the test above.

Due to the different PCIe controller have different property, e.g. PCIe 
controller1 support
multiple function feature, but PCIe controller2 don't support this feature, so 
I need to check
which controller support it and return the correct offset value, but each board 
only have one
ls_pcie_ep_drvdata, ^_^.

> 
> However something to the effect of the following may help spot
> misconfiguration:
> 
> WARN_ON(func_no && !pcie->drvdata->func_offset); return
> pcie->drvdata->func_offset * func_no;
> 
> The WARN is probably quite useful as if you are attempting to use non-zero
> functions and func_offset isn't set - then things may appear to work normally
> but actually will break horribly.

As disc

RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-24 Thread Xiaowei Bao


> -Original Message-
> From: christophe leroy 
> Sent: 2019年8月24日 14:45
> To: Xiaowei Bao ; Andrew Murray
> 
> Cc: mark.rutl...@arm.com; Roy Zang ;
> lorenzo.pieral...@arm.co; a...@arndb.de; devicet...@vger.kernel.org;
> gre...@linuxfoundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-...@vger.kernel.org; linux-ker...@vger.kernel.org; kis...@ti.com; M.h.
> Lian ; robh...@kernel.org;
> gustavo.pimen...@synopsys.com; jingooh...@gmail.com;
> bhelg...@google.com; Leo Li ; shawn...@kernel.org;
> Mingkai Hu ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
> 
> 
> 
> Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :
> >
> >
> >> -Original Message-
> >> From: Andrew Murray 
> >> Sent: 2019年8月23日 22:28
> >> To: Xiaowei Bao 
> >> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> >> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> >> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org;
> M.h.
> >> Lian ; Mingkai Hu ; Roy
> >> Zang ; jingooh...@gmail.com;
> >> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support
> >> for ls1088a and ls2088a
> >>
> >> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> >>> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> >>> difference between LS1 and LS2 platform, so refactor the code of the
> >>> EP driver.
> >>>
> >>> Signed-off-by: Xiaowei Bao 
> >>> ---
> >>> v2:
> >>>   - New mechanism for layerscape EP driver.
> >>
> >> Was there a v1 of this patch?
> >
> > Yes, but I don't know how to comments, ^_^
> 
> As far as I can see, in the previous version of the series
> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fproject%2Flinuxppc-dev%2Flist%2F%3Fseries%3D125315
> %26state%3D*data=02%7C01%7Cxiaowei.bao%40nxp.com%7C1befe9
> a67c8046f9535e08d7285eaab6%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C637022259387139020sdata=p4wbycd04Z7qRUfAoZtwc
> UP7pR%2FuA3%2FjVcWMz6YyQVQ%3Dreserved=0),
> the 8/10 was something completely different, and I can't find any other patch
> in the series that could have been the v1 of this patch.

Thanks, I will correct it to v1 in next version patch.

> 
> Christophe
> 
> >
> >>
> >>>
> >>>   drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> >>> --
> >>>   1 file changed, 58 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> index 7ca5fe8..2a66f07 100644
> >>> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> @@ -20,27 +20,29 @@
> >>>
> >>>   #define PCIE_DBI2_OFFSET0x1000  /* DBI2 base address*/
> >>>
> >>> -struct ls_pcie_ep {
> >>> - struct dw_pcie  *pci;
> >>> - struct pci_epc_features *ls_epc;
> >>> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >>> +
> >>> +struct ls_pcie_ep_drvdata {
> >>> + u32 func_offset;
> >>> + const struct dw_pcie_ep_ops *ops;
> >>> + const struct dw_pcie_ops*dw_pcie_ops;
> >>>   };
> >>>
> >>> -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >>> +struct ls_pcie_ep {
> >>> + struct dw_pcie  *pci;
> >>> + struct pci_epc_features *ls_epc;
> >>> + const struct ls_pcie_ep_drvdata *drvdata; };
> >>>
> >>>   static int ls_pcie_establish_link(struct dw_pcie *pci)  {
> >>>   return 0;
> >>>   }
> >>>
> >>> -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> >>> +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> >>>   .start_link = ls_pcie_establish_link,  };
> >>>
> >>> -static const struct of_device_id ls_pcie_ep_of_match[] = {
> >>> - { .compatible = "fsl,ls-pcie-ep",},
> &g

Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-24 Thread christophe leroy




Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :




-Original Message-
From: Andrew Murray 
Sent: 2019年8月23日 22:28
To: Xiaowei Bao 
Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
shawn...@kernel.org; Leo Li ; kis...@ti.com;
lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
Lian ; Mingkai Hu ; Roy
Zang ; jingooh...@gmail.com;
gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
ls1088a and ls2088a

On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:

Add PCIe EP mode support for ls1088a and ls2088a, there are some
difference between LS1 and LS2 platform, so refactor the code of the
EP driver.

Signed-off-by: Xiaowei Bao 
---
v2:
  - New mechanism for layerscape EP driver.


Was there a v1 of this patch?


Yes, but I don't know how to comments, ^_^


As far as I can see, in the previous version of the series 
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=125315=*), 
the 8/10 was something completely different, and I can't find any other 
patch in the series that could have been the v1 of this patch.


Christophe







  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
--
  1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 7ca5fe8..2a66f07 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -20,27 +20,29 @@

  #define PCIE_DBI2_OFFSET  0x1000  /* DBI2 base address*/

-struct ls_pcie_ep {
-   struct dw_pcie  *pci;
-   struct pci_epc_features *ls_epc;
+#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
+
+struct ls_pcie_ep_drvdata {
+   u32 func_offset;
+   const struct dw_pcie_ep_ops *ops;
+   const struct dw_pcie_ops*dw_pcie_ops;
  };

-#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
+struct ls_pcie_ep {
+   struct dw_pcie  *pci;
+   struct pci_epc_features *ls_epc;
+   const struct ls_pcie_ep_drvdata *drvdata; };

  static int ls_pcie_establish_link(struct dw_pcie *pci)  {
return 0;
  }

-static const struct dw_pcie_ops ls_pcie_ep_ops = {
+static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
.start_link = ls_pcie_establish_link,  };

-static const struct of_device_id ls_pcie_ep_of_match[] = {
-   { .compatible = "fsl,ls-pcie-ep",},
-   { },
-};
-
  static const struct pci_epc_features*  ls_pcie_ep_get_features(struct
dw_pcie_ep *ep)  { @@ -82,10 +84,44 @@ static int
ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
}
  }

-static const struct dw_pcie_ep_ops pcie_ep_ops = {
+static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
+   u8 func_no)
+{
+   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+   struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
+   u8 header_type;
+
+   header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
+
+   if (header_type & (1 << 7))
+   return pcie->drvdata->func_offset * func_no;
+   else
+   return 0;


It looks like there isn't a PCI define for multi function, the nearest I could 
find
was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
above the test might be helpful to explain the test.


Yes, I have not find the PCI_HEADER_TYPE_MULTIDEVICE define. OK, I will add
The comments in next version patch.



As the ls_pcie_ep_drvdata structures are static, the unset .func_offset will be
initialised to 0, so you could just drop the test above.


OK, thanks



However something to the effect of the following may help spot
misconfiguration:

WARN_ON(func_no && !pcie->drvdata->func_offset); return
pcie->drvdata->func_offset * func_no;


Thanks a lot, this looks better.



The WARN is probably quite useful as if you are attempting to use non-zero
functions and func_offset isn't set - then things may appear to work normally
but actually will break horribly.


got it, thanks.



Thanks,

Andrew Murray


+}
+
+static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
.ep_init = ls_pcie_ep_init,
.raise_irq = ls_pcie_ep_raise_irq,
.get_features = ls_pcie_ep_get_features,
+   .func_conf_select = ls_pcie_ep_func_conf_select, };
+
+static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
+   .ops = _pcie_ep_ops,
+   .dw_pcie_ops = _ls_pcie_ep_ops,
+};
+
+static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
+   .func_offset = 0x2,
+   .ops = _pcie_ep_ops,
+   .dw_pcie_ops = _ls_pcie_ep_ops,
+};
+
+static const struct of_de

RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-23 Thread Xiaowei Bao


> -Original Message-
> From: Andrew Murray 
> Sent: 2019年8月23日 22:28
> To: Xiaowei Bao 
> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
> Lian ; Mingkai Hu ; Roy
> Zang ; jingooh...@gmail.com;
> gustavo.pimen...@synopsys.com; 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 v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
> 
> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> > Add PCIe EP mode support for ls1088a and ls2088a, there are some
> > difference between LS1 and LS2 platform, so refactor the code of the
> > EP driver.
> >
> > Signed-off-by: Xiaowei Bao 
> > ---
> > v2:
> >  - New mechanism for layerscape EP driver.
> 
> Was there a v1 of this patch?

Yes, but I don't know how to comments, ^_^

> 
> >
> >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> > --
> >  1 file changed, 58 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index 7ca5fe8..2a66f07 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -20,27 +20,29 @@
> >
> >  #define PCIE_DBI2_OFFSET   0x1000  /* DBI2 base address*/
> >
> > -struct ls_pcie_ep {
> > -   struct dw_pcie  *pci;
> > -   struct pci_epc_features *ls_epc;
> > +#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > +
> > +struct ls_pcie_ep_drvdata {
> > +   u32 func_offset;
> > +   const struct dw_pcie_ep_ops *ops;
> > +   const struct dw_pcie_ops*dw_pcie_ops;
> >  };
> >
> > -#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > +struct ls_pcie_ep {
> > +   struct dw_pcie  *pci;
> > +   struct pci_epc_features *ls_epc;
> > +   const struct ls_pcie_ep_drvdata *drvdata; };
> >
> >  static int ls_pcie_establish_link(struct dw_pcie *pci)  {
> > return 0;
> >  }
> >
> > -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> > +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> > .start_link = ls_pcie_establish_link,  };
> >
> > -static const struct of_device_id ls_pcie_ep_of_match[] = {
> > -   { .compatible = "fsl,ls-pcie-ep",},
> > -   { },
> > -};
> > -
> >  static const struct pci_epc_features*  ls_pcie_ep_get_features(struct
> > dw_pcie_ep *ep)  { @@ -82,10 +84,44 @@ static int
> > ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> > }
> >  }
> >
> > -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> > +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> > +   u8 func_no)
> > +{
> > +   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +   struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > +   u8 header_type;
> > +
> > +   header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> > +
> > +   if (header_type & (1 << 7))
> > +   return pcie->drvdata->func_offset * func_no;
> > +   else
> > +   return 0;
> 
> It looks like there isn't a PCI define for multi function, the nearest I 
> could find
> was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
> above the test might be helpful to explain the test.

Yes, I have not find the PCI_HEADER_TYPE_MULTIDEVICE define. OK, I will add
The comments in next version patch.

> 
> As the ls_pcie_ep_drvdata structures are static, the unset .func_offset will 
> be
> initialised to 0, so you could just drop the test above.

OK, thanks

> 
> However something to the effect of the following may help spot
> misconfiguration:
> 
> WARN_ON(func_no && !pcie->drvdata->func_offset); return
> pcie->drvdata->func_offset * func_no;

Thanks a lot, this looks better.

> 
> The WARN is probably quite useful as if you are attempting to use non-zero
> functions and func_offset isn't set - then things may appear to work normally
> but actually will break horribly.

got it, thanks.

> 
> Thanks,
> 
> Andrew Murray
> 
> > +}
> > +
> > +static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
> > 

Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a

2019-08-23 Thread Andrew Murray
On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> difference between LS1 and LS2 platform, so refactor the code of
> the EP driver.
> 
> Signed-off-by: Xiaowei Bao 
> ---
> v2:
>  - New mechanism for layerscape EP driver.

Was there a v1 of this patch?

> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 76 
> --
>  1 file changed, 58 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index 7ca5fe8..2a66f07 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -20,27 +20,29 @@
>  
>  #define PCIE_DBI2_OFFSET 0x1000  /* DBI2 base address*/
>  
> -struct ls_pcie_ep {
> - struct dw_pcie  *pci;
> - struct pci_epc_features *ls_epc;
> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> +
> +struct ls_pcie_ep_drvdata {
> + u32 func_offset;
> + const struct dw_pcie_ep_ops *ops;
> + const struct dw_pcie_ops*dw_pcie_ops;
>  };
>  
> -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> +struct ls_pcie_ep {
> + struct dw_pcie  *pci;
> + struct pci_epc_features *ls_epc;
> + const struct ls_pcie_ep_drvdata *drvdata;
> +};
>  
>  static int ls_pcie_establish_link(struct dw_pcie *pci)
>  {
>   return 0;
>  }
>  
> -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
>   .start_link = ls_pcie_establish_link,
>  };
>  
> -static const struct of_device_id ls_pcie_ep_of_match[] = {
> - { .compatible = "fsl,ls-pcie-ep",},
> - { },
> -};
> -
>  static const struct pci_epc_features*
>  ls_pcie_ep_get_features(struct dw_pcie_ep *ep)
>  {
> @@ -82,10 +84,44 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 
> func_no,
>   }
>  }
>  
> -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> + u8 func_no)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> + u8 header_type;
> +
> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> +
> + if (header_type & (1 << 7))
> + return pcie->drvdata->func_offset * func_no;
> + else
> + return 0;

It looks like there isn't a PCI define for multi function, the nearest I
could find was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
above the test might be helpful to explain the test.

As the ls_pcie_ep_drvdata structures are static, the unset .func_offset
will be initialised to 0, so you could just drop the test above.

However something to the effect of the following may help spot
misconfiguration:

WARN_ON(func_no && !pcie->drvdata->func_offset);
return pcie->drvdata->func_offset * func_no;

The WARN is probably quite useful as if you are attempting to use
non-zero functions and func_offset isn't set - then things may appear to work
normally but actually will break horribly.

Thanks,

Andrew Murray

> +}
> +
> +static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
>   .ep_init = ls_pcie_ep_init,
>   .raise_irq = ls_pcie_ep_raise_irq,
>   .get_features = ls_pcie_ep_get_features,
> + .func_conf_select = ls_pcie_ep_func_conf_select,
> +};
> +
> +static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
> + .ops = _pcie_ep_ops,
> + .dw_pcie_ops = _ls_pcie_ep_ops,
> +};
> +
> +static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
> + .func_offset = 0x2,
> + .ops = _pcie_ep_ops,
> + .dw_pcie_ops = _ls_pcie_ep_ops,
> +};
> +
> +static const struct of_device_id ls_pcie_ep_of_match[] = {
> + { .compatible = "fsl,ls1046a-pcie-ep", .data = _ep_drvdata },
> + { .compatible = "fsl,ls1088a-pcie-ep", .data = _ep_drvdata },
> + { .compatible = "fsl,ls2088a-pcie-ep", .data = _ep_drvdata },
> + { },
>  };
>  
>  static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> @@ -98,7 +134,7 @@ static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
>   int ret;
>  
>   ep = >ep;
> - ep->ops = _ep_ops;
> + ep->ops = pcie->drvdata->ops;
>  
>   res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
>   if (!res)
> @@ -137,14 +173,11 @@ static int __init ls_pcie_ep_probe(struct 
> platform_device *pdev)
>   if (!ls_epc)
>   return -ENOMEM;
>  
> - dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM, "regs");
> - pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> - if (IS_ERR(pci->dbi_base))
> - return PTR_ERR(pci->dbi_base);
> + pcie->drvdata = of_device_get_match_data(dev);
>  
> - pci->dbi_base2 = pci->dbi_base + PCIE_DBI2_OFFSET;
>