Re: [PATCH v6 0/4] PCI: layerscape: Add suspend/resume support for ls1043 and ls1021

[Lorenzo Pieralisi]

On Mon, 04 Dec 2023 11:08:25 -0500, Frank Li wrote:
> Add suspend/resume support for ls1043 and ls1021.
> 
> Change log see each patch
> 
> Frank Li (4):
>   PCI: layerscape: Add function pointer for exit_from_l2()
>   PCI: layerscape: Add suspend/resume for ls1021a
>   PCI: layerscape(ep): Rename pf_* as pf_lut_*
>   PCI: layerscape: Add suspend/resume for ls1043a
> 
> [...]

Applied to controller/layerscape, thanks!

[1/4] PCI: layerscape: Add function pointer for exit_from_l2()
  https://git.kernel.org/pci/pci/c/123971a193d9
[2/4] PCI: layerscape: Add suspend/resume for ls1021a
  https://git.kernel.org/pci/pci/c/6f8a41ba2623
[3/4] PCI: layerscape(ep): Rename pf_* as pf_lut_*
  https://git.kernel.org/pci/pci/c/762ef94b45d9
[4/4] PCI: layerscape: Add suspend/resume for ls1043a
  https://git.kernel.org/pci/pci/c/27b3bcbf8a79

Thanks,
Lorenzo

Re: [PATCH 2/3] PCI: layerscape: add suspend/resume for ls1021a

[Lorenzo Pieralisi]

On Mon, Oct 16, 2023 at 12:11:04PM -0400, Frank Li wrote:
> On Mon, Oct 16, 2023 at 10:22:11AM -0500, Bjorn Helgaas wrote:
> > On Mon, Oct 16, 2023 at 10:45:25AM -0400, Frank Li wrote:
> > > On Tue, Oct 10, 2023 at 06:02:36PM +0200, Lorenzo Pieralisi wrote:
> > > > On Tue, Oct 10, 2023 at 10:20:12AM -0400, Frank Li wrote:
> > 
> > > > > Ping
> > > > 
> > > > Read and follow please (and then ping us):
> > > > https://lore.kernel.org/linux-pci/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com
> > > 
> > > Could you please help point which specic one was not follow aboved guide?
> > > Then I can update my code. I think that's efficial communication method. I
> > > think I have read it serial times. But not sure which one violate the
> > > guide?
> > > 
> > > @Bjorn Helgaas. How do you think so? 
> > 
> > Since Lorenzo didn't point out anything specific in the patch itself,
> > I think he was probably referring to the subject line and this advice:
> > 
> >   - Follow the existing convention, i.e., run "git log --oneline
> > " and make yours match in format, capitalization, and
> > sentence structure.  For example, native host bridge driver patch
> > titles look like this:
> > 
> >   PCI: altera: Fix platform_get_irq() error handling
> >   PCI: vmd: Remove IRQ affinity so we can allocate more IRQs
> >   PCI: mediatek: Add MSI support for MT2712 and MT7622
> >   PCI: rockchip: Remove IRQ domain if probe fails
> > 
> > In this case, your subject line was:
> > 
> >   PCI: layerscape: add suspend/resume for ls1021a
> > 
> > The advice was to run this:
> > 
> >   $ git log --oneline drivers/pci/controller/dwc/pci-layerscape.c
> >   83c088148c8e PCI: Use PCI_HEADER_TYPE_* instead of literals
> >   9fda4d09905d PCI: layerscape: Add power management support for ls1028a
> >   277004d7a4a3 PCI: Remove unnecessary  includes
> >   60b3c27fb9b9 PCI: dwc: Rename struct pcie_port to dw_pcie_rp
> >   d23f0c11aca2 PCI: layerscape: Change to use the DWC common link-up check 
> > function
> >   7007b745a508 PCI: layerscape: Convert to builtin_platform_driver()
> >   60f5b73fa0f2 PCI: dwc: Remove unnecessary wrappers around 
> > dw_pcie_host_init()
> >   b9ac0f9dc8ea PCI: dwc: Move dw_pcie_setup_rc() to DWC common code
> >   f78f02638af5 PCI: dwc: Rework MSI initialization
> > 
> > Note that these summaries are all complete sentences that start with a
> > capital letter:
> > 
> >   Use PCI_HEADER_TYPE_* instead of literals
> >   Add power management support for ls1028a
> >   Remove unnecessary  includes
> >   ...
> > 
> > So yours could be this:
> > 
> >   PCI: layerscape: Add suspend/resume for ls1021a
> >^
> > 
> > This is trivial, obviously.  But the uppercase/lowercase distinction
> > carries information, and it's an unnecessary distraction to notice
> > that "oh, this is different from the rest; is the difference
> > important or should I ignore it?"
> 
> Thanks. Not everyone think it is trivial. Especially for the one, who
> English are not native language.
> 
> > 
> > Obviously Lorenzo *could* edit all your subject lines on your behalf,
> > but it makes everybody's life easier if people look at the existing
> > code and follow the style when making changes.
> 
> Understand, but simple mark 'a' and 'A' to me. I will update patches and
> take care for next time instead search whole docuemnt to guess which one
> violated. I know I make some mistakes at here. But I am working on many
> difference kernel subsystems, some require upper case, some require low
> case, someone doesn't care.
>  
> I respected all reviewer's and maintaner's time, but I hope respected
> the contributor's time also.

I do respect your time, please respect ours by following the guidelines I
provided you with multiple times already.

Thank you for resending the series.

Lorenzo

Re: [PATCH 2/3] PCI: layerscape: add suspend/resume for ls1021a

[Lorenzo Pieralisi]

On Mon, Oct 16, 2023 at 10:22:11AM -0500, Bjorn Helgaas wrote:
> On Mon, Oct 16, 2023 at 10:45:25AM -0400, Frank Li wrote:
> > On Tue, Oct 10, 2023 at 06:02:36PM +0200, Lorenzo Pieralisi wrote:
> > > On Tue, Oct 10, 2023 at 10:20:12AM -0400, Frank Li wrote:
> 
> > > > Ping
> > > 
> > > Read and follow please (and then ping us):
> > > https://lore.kernel.org/linux-pci/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com
> > 
> > Could you please help point which specic one was not follow aboved guide?
> > Then I can update my code. I think that's efficial communication method. I
> > think I have read it serial times. But not sure which one violate the
> > guide?
> > 
> > @Bjorn Helgaas. How do you think so? 
> 
> Since Lorenzo didn't point out anything specific in the patch itself,
> I think he was probably referring to the subject line and this advice:
> 
>   - Follow the existing convention, i.e., run "git log --oneline
> " and make yours match in format, capitalization, and
> sentence structure.  For example, native host bridge driver patch
> titles look like this:
> 
>   PCI: altera: Fix platform_get_irq() error handling
>   PCI: vmd: Remove IRQ affinity so we can allocate more IRQs
>   PCI: mediatek: Add MSI support for MT2712 and MT7622
>   PCI: rockchip: Remove IRQ domain if probe fails
> 
> In this case, your subject line was:
> 
>   PCI: layerscape: add suspend/resume for ls1021a
> 
> The advice was to run this:
> 
>   $ git log --oneline drivers/pci/controller/dwc/pci-layerscape.c
>   83c088148c8e PCI: Use PCI_HEADER_TYPE_* instead of literals
>   9fda4d09905d PCI: layerscape: Add power management support for ls1028a
>   277004d7a4a3 PCI: Remove unnecessary  includes
>   60b3c27fb9b9 PCI: dwc: Rename struct pcie_port to dw_pcie_rp
>   d23f0c11aca2 PCI: layerscape: Change to use the DWC common link-up check 
> function
>   7007b745a508 PCI: layerscape: Convert to builtin_platform_driver()
>   60f5b73fa0f2 PCI: dwc: Remove unnecessary wrappers around 
> dw_pcie_host_init()
>   b9ac0f9dc8ea PCI: dwc: Move dw_pcie_setup_rc() to DWC common code
>   f78f02638af5 PCI: dwc: Rework MSI initialization
> 
> Note that these summaries are all complete sentences that start with a
> capital letter:
> 
>   Use PCI_HEADER_TYPE_* instead of literals
>   Add power management support for ls1028a
>   Remove unnecessary  includes
>   ...
> 
> So yours could be this:
> 
>   PCI: layerscape: Add suspend/resume for ls1021a
>^
> 
> This is trivial, obviously.  But the uppercase/lowercase distinction
> carries information, and it's an unnecessary distraction to notice
> that "oh, this is different from the rest; is the difference
> important or should I ignore it?"
> 
> Obviously Lorenzo *could* edit all your subject lines on your behalf,
> but it makes everybody's life easier if people look at the existing
> code and follow the style when making changes.
> 
> E.g., write subject lines that are similar in style to previous ones,
> name local variables similarly to other functions, use line lengths
> consistent with the rest of the file, etc.  After applying a change,
> the file should look like a coherent whole; we should not be able to
> tell that this hunk was added later by somebody else.  This all helps
> make the code (and the git history) more readable and maintainable.

Thank you very much Bjorn. Frank, now please resend your patches and
make sure that you follow these guidelines, they must be clear by now.

Lorenzo

Re: [PATCH 2/3] PCI: layerscape: add suspend/resume for ls1021a

[Lorenzo Pieralisi]

On Tue, Oct 10, 2023 at 10:20:12AM -0400, Frank Li wrote:
> On Wed, Oct 04, 2023 at 10:23:51AM -0400, Frank Li wrote:
> > On Fri, Sep 15, 2023 at 02:43:05PM -0400, Frank Li wrote:
> > > ls1021a add suspend/resume support.
> > > 
> > > Signed-off-by: Frank Li 
> > > ---
> > 
> > ping
> > 
> > Frank
> 
> Ping

Read and follow please (and then ping us):
https://lore.kernel.org/linux-pci/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com

> Frank
> 
> > 
> > >  drivers/pci/controller/dwc/pci-layerscape.c | 88 -
> > >  1 file changed, 87 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c 
> > > b/drivers/pci/controller/dwc/pci-layerscape.c
> > > index 20c48c06e2248..bc5a8ff1a26ce 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > > @@ -35,6 +35,12 @@
> > >  #define PF_MCR_PTOMR BIT(0)
> > >  #define PF_MCR_EXL2S BIT(1)
> > >  
> > > +/* LS1021A PEXn PM Write Control Register */
> > > +#define SCFG_PEXPMWRCR(idx)  (0x5c + (idx) * 0x64)
> > > +#define PMXMTTURNOFF BIT(31)
> > > +#define SCFG_PEXSFTRSTCR 0x190
> > > +#define PEXSR(idx)   BIT(idx)
> > > +
> > >  #define PCIE_IATU_NUM6
> > >  
> > >  struct ls_pcie_drvdata {
> > > @@ -48,6 +54,8 @@ struct ls_pcie {
> > >   struct dw_pcie *pci;
> > >   const struct ls_pcie_drvdata *drvdata;
> > >   void __iomem *pf_base;
> > > + struct regmap *scfg;
> > > + int index;
> > >   bool big_endian;
> > >  };
> > >  
> > > @@ -170,13 +178,91 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
> > >   return 0;
> > >  }
> > >  
> > > +static void ls1021a_pcie_send_turnoff_msg(struct dw_pcie_rp *pp)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > + struct ls_pcie *pcie = to_ls_pcie(pci);
> > > + u32 val;
> > > +
> > > + if (!pcie->scfg) {
> > > + dev_dbg(pcie->pci->dev, "SYSCFG is NULL\n");
> > > + return;
> > > + }
> > > +
> > > + /* Send Turn_off message */
> > > + regmap_read(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), );
> > > + val |= PMXMTTURNOFF;
> > > + regmap_write(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), val);
> > > +
> > > + /* There are not register to check ACK, so wait 
> > > PCIE_PME_TO_L2_TIMEOUT_US */
> > > + mdelay(PCIE_PME_TO_L2_TIMEOUT_US/1000);
> > > +
> > > + /* Clear Turn_off message */
> > > + regmap_read(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), );
> > > + val &= ~PMXMTTURNOFF;
> > > + regmap_write(pcie->scfg, SCFG_PEXPMWRCR(pcie->index), val);
> > > +}
> > > +
> > > +static void ls1021a_pcie_exit_from_l2(struct dw_pcie_rp *pp)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > + struct ls_pcie *pcie = to_ls_pcie(pci);
> > > + u32 val;
> > > +
> > > + regmap_read(pcie->scfg, SCFG_PEXSFTRSTCR, );
> > > + val |= PEXSR(pcie->index);
> > > + regmap_write(pcie->scfg, SCFG_PEXSFTRSTCR, val);
> > > +
> > > + regmap_read(pcie->scfg, SCFG_PEXSFTRSTCR, );
> > > + val &= ~PEXSR(pcie->index);
> > > + regmap_write(pcie->scfg, SCFG_PEXSFTRSTCR, val);
> > > +}
> > > +
> > > +static int ls1021a_pcie_host_init(struct dw_pcie_rp *pp)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > > + struct ls_pcie *pcie = to_ls_pcie(pci);
> > > + struct device *dev = pcie->pci->dev;
> > > + u32 index[2];
> > > + int ret;
> > > +
> > > + ret = ls_pcie_host_init(pp);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + pcie->scfg = syscon_regmap_lookup_by_phandle(dev->of_node, 
> > > "fsl,pcie-scfg");
> > > + if (IS_ERR(pcie->scfg)) {
> > > + ret = PTR_ERR(pcie->scfg);
> > > + dev_err(dev, "No syscfg phandle specified\n");
> > > + pcie->scfg = NULL;
> > > + return ret;
> > > + }
> > > +
> > > + ret = of_property_read_u32_array(dev->of_node, "fsl,pcie-scfg", index, 
> > > 2);
> > > + if (ret) {
> > > + pcie->scfg = NULL;
> > > + return ret;
> > > + }
> > > +
> > > + pcie->index = index[1];
> > > +
> > > + return ret;
> > > +}
> > > +
> > >  static const struct dw_pcie_host_ops ls_pcie_host_ops = {
> > >   .host_init = ls_pcie_host_init,
> > >   .pme_turn_off = ls_pcie_send_turnoff_msg,
> > >  };
> > >  
> > > +static const struct dw_pcie_host_ops ls1021a_pcie_host_ops = {
> > > + .host_init = ls1021a_pcie_host_init,
> > > + .pme_turn_off = ls1021a_pcie_send_turnoff_msg,
> > > +};
> > > +
> > >  static const struct ls_pcie_drvdata ls1021a_drvdata = {
> > > - .pm_support = false,
> > > + .pm_support = true,
> > > + .ops = _pcie_host_ops,
> > > + .exit_from_l2 = ls1021a_pcie_exit_from_l2,
> > >  };
> > >  
> > >  static const struct ls_pcie_drvdata layerscape_drvdata = {
> > > -- 
> > > 2.34.1
> > > 

Re: [PATCH v3 1/1] PCI: layerscape-ep: set 64-bit DMA mask

[Lorenzo Pieralisi]

On Tue, 26 Sep 2023 10:04:45 -0400, Frank Li wrote:
> Set DMA mask and coherent DMA mask to enable 64-bit addressing.
> 
> 

Read this:
https://lore.kernel.org/linux-pci/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com

Find the issue with the commit log (that I fixed).

This does not apply to v6.6-rc1 so I tweaked it,
check that everything is OK please.

Applied to controller/layerscape, thanks!

[1/1] PCI: layerscape-ep: set 64-bit DMA mask
  https://git.kernel.org/pci/pci/c/81ef01bc5934

Thanks,
Lorenzo

Re: [PATCH v4 1/2] PCI: layerscape: Add support for Link down notification

[Lorenzo Pieralisi]

On Thu, 20 Jul 2023 09:58:33 -0400, Frank Li wrote:
> Add support to pass Link down notification to Endpoint function driver
> so that the LINK_DOWN event can be processed by the function.
> 
> 

Applied to controller/layerscape, thanks!

[1/2] PCI: layerscape: Add support for Link down notification
  https://git.kernel.org/pci/pci/c/d28c0d84ca40
[2/2] PCI: layerscape: Add the workaround for lost link capabilities during 
reset.
  https://git.kernel.org/pci/pci/c/17cf8661ee0f

Thanks,
Lorenzo

Re: [PATCH v4 1/2] PCI: layerscape: Add support for Link down notification

[Lorenzo Pieralisi]

On Wed, Aug 16, 2023 at 11:53:16AM -0400, Frank Li wrote:
> On Mon, Jul 31, 2023 at 11:06:31AM -0400, Frank Li wrote:
> > On Thu, Jul 20, 2023 at 09:58:33AM -0400, Frank Li wrote:
> > > Add support to pass Link down notification to Endpoint function driver
> > > so that the LINK_DOWN event can be processed by the function.
> > > 
> > > Acked-by: Manivannan Sadhasivam 
> > > Signed-off-by: Frank Li 
> > > ---
> > 
> > @Lorenzo
> > 
> > Could you please consider pick both patches?
> > Manivannan already reviewed and only impact layerscape.
> 
> @lorenzo:
>   ping

I will have a look tomorrow.

Lorenzo

> > 
> > Frank
> > 
> > > Change from v2 to v4
> > >  - none
> > > Change from v1 to v2
> > > 
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index de4c1758a6c3..e0969ff2ddf7 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -89,6 +89,7 @@ static irqreturn_t ls_pcie_ep_event_handler(int irq, 
> > > void *dev_id)
> > >   dev_dbg(pci->dev, "Link up\n");
> > >   } else if (val & PEX_PF0_PME_MES_DR_LDD) {
> > >   dev_dbg(pci->dev, "Link down\n");
> > > + pci_epc_linkdown(pci->ep.epc);
> > >   } else if (val & PEX_PF0_PME_MES_DR_HRD) {
> > >   dev_dbg(pci->dev, "Hot reset\n");
> > >   }
> > > -- 
> > > 2.34.1
> > > 

Re: [PATCH v4 1/1] PCI: layerscape: Add the endpoint linkup notifier support

[Lorenzo Pieralisi]

On Mon, 15 May 2023 11:10:49 -0400, Frank Li wrote:
> Layerscape has PME interrupt, which can be used as linkup notifier.
> Set CFG_READY bit of PEX_PF0_CONFIG to enable accesses from root complex
> when linkup detected.
> 
> 

Applied to controller/endpoint, thanks!

[1/1] PCI: layerscape: Add the endpoint linkup notifier support
  https://git.kernel.org/pci/pci/c/71d9ca28015c

Thanks,
Lorenzo

Re: [PATCH v4 1/1] PCI: layerscape: Add the endpoint linkup notifier support

[Lorenzo Pieralisi]

On Mon, May 15, 2023 at 11:10:49AM -0400, Frank Li wrote:
> Layerscape has PME interrupt, which can be used as linkup notifier.
> Set CFG_READY bit of PEX_PF0_CONFIG to enable accesses from root complex
> when linkup detected.
> 
> Acked-by: Manivannan Sadhasivam 
> Signed-off-by: Xiaowei Bao 
> Signed-off-by: Frank Li 
> ---
> Change from v3 to v4
>  - swap irq and big_endian
> Change from v2 to v3
>  - align 80 column
>  - clear irq firstly
>  - dev_info to dev_dbg
>  - remove double space
>  - update commit message
> 
> Change from v1 to v2
> - pme -> PME
> - irq -> IRQ
> - update dev_info message according to Bjorn's suggestion
> 
>  .../pci/controller/dwc/pci-layerscape-ep.c| 102 +-
>  1 file changed, 101 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index c640db60edc6..5398641b6b7e 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -18,6 +18,20 @@
>  
>  #include "pcie-designware.h"
>  
> +#define PEX_PF0_CONFIG   0xC0014
> +#define PEX_PF0_CFG_READYBIT(0)
> +
> +/* PEX PFa PCIE PME and message interrupt registers*/
> +#define PEX_PF0_PME_MES_DR   0xC0020
> +#define PEX_PF0_PME_MES_DR_LUD   BIT(7)
> +#define PEX_PF0_PME_MES_DR_LDD   BIT(9)
> +#define PEX_PF0_PME_MES_DR_HRD   BIT(10)
> +
> +#define PEX_PF0_PME_MES_IER  0xC0028
> +#define PEX_PF0_PME_MES_IER_LUDIEBIT(7)
> +#define PEX_PF0_PME_MES_IER_LDDIEBIT(9)
> +#define PEX_PF0_PME_MES_IER_HRDIEBIT(10)
> +
>  #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>  
>  struct ls_pcie_ep_drvdata {
> @@ -30,8 +44,86 @@ struct ls_pcie_ep {
>   struct dw_pcie  *pci;
>   struct pci_epc_features *ls_epc;
>   const struct ls_pcie_ep_drvdata *drvdata;
> + int irq;
> + boolbig_endian;
>  };
>  
> +static u32 ls_lut_readl(struct ls_pcie_ep *pcie, u32 offset)
> +{
> + struct dw_pcie *pci = pcie->pci;
> +
> + if (pcie->big_endian)
> + return ioread32be(pci->dbi_base + offset);
> + else
> + return ioread32(pci->dbi_base + offset);
> +}
> +
> +static void ls_lut_writel(struct ls_pcie_ep *pcie, u32 offset, u32 value)
> +{
> + struct dw_pcie *pci = pcie->pci;
> +
> + if (pcie->big_endian)
> + iowrite32be(value, pci->dbi_base + offset);
> + else
> + iowrite32(value, pci->dbi_base + offset);
> +}
> +
> +static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
> +{
> + struct ls_pcie_ep *pcie = dev_id;
> + struct dw_pcie *pci = pcie->pci;
> + u32 val, cfg;
> +
> + val = ls_lut_readl(pcie, PEX_PF0_PME_MES_DR);
> + ls_lut_writel(pcie, PEX_PF0_PME_MES_DR, val);
> +
> + if (!val)
> + return IRQ_NONE;
> +
> + if (val & PEX_PF0_PME_MES_DR_LUD) {
> + cfg = ls_lut_readl(pcie, PEX_PF0_CONFIG);
> + cfg |= PEX_PF0_CFG_READY;
> + ls_lut_writel(pcie, PEX_PF0_CONFIG, cfg);
> + dw_pcie_ep_linkup(>ep);
> +
> + dev_dbg(pci->dev, "Link up\n");
> + } else if (val & PEX_PF0_PME_MES_DR_LDD) {
> + dev_dbg(pci->dev, "Link down\n");
> + } else if (val & PEX_PF0_PME_MES_DR_HRD) {
> + dev_dbg(pci->dev, "Hot reset\n");
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
> +  struct platform_device *pdev)
> +{
> + u32 val;
> + int ret;
> +
> + pcie->irq = platform_get_irq_byname(pdev, "pme");
> + if (pcie->irq < 0) {
> + dev_err(>dev, "Can't get 'pme' IRQ\n");

I will remove this log, platform_get_irq_byname() already logs an
error.

Lorenzo

> + return pcie->irq;
> + }
> +
> + ret = devm_request_irq(>dev, pcie->irq, ls_pcie_ep_event_handler,
> +IRQF_SHARED, pdev->name, pcie);
> + if (ret) {
> + dev_err(>dev, "Can't register PCIe IRQ\n");
> + return ret;
> + }
> +
> + /* Enable interrupts */
> + val = ls_lut_readl(pcie, PEX_PF0_PME_MES_IER);
> + val |=  PEX_PF0_PME_MES_IER_LDDIE | PEX_PF0_PME_MES_IER_HRDIE |
> + PEX_PF0_PME_MES_IER_LUDIE;
> + ls_lut_writel(pcie, PEX_PF0_PME_MES_IER, val);
> +
> + return 0;
> +}
> +
>  static const struct pci_epc_features*
>  ls_pcie_ep_get_features(struct dw_pcie_ep *ep)
>  {
> @@ -125,6 +217,7 @@ static int __init ls_pcie_ep_probe(struct platform_device 
> *pdev)
>   struct ls_pcie_ep *pcie;
>   struct pci_epc_features *ls_epc;
>   struct resource *dbi_base;
> + int ret;
>  
>   pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>   if (!pcie)
> @@ -144,6 +237,7 @@ static int __init ls_pcie_ep_probe(struct 

Re: [PATCH v3 1/1] PCI: layerscape: Add EP mode support for ls1028a

[Lorenzo Pieralisi]

On Thu, 09 Feb 2023 10:10:50 -0500, Frank Li wrote:
> Add PCIe EP mode support for ls1028a.
> 
> 

Applied to controller/layerscape, thanks!

[1/1] PCI: layerscape: Add EP mode support for ls1028a
  https://git.kernel.org/pci/pci/c/be567c6cbc08

Thanks,
Lorenzo

Re: [PATCH 1/1] PCI: layerscape: Add EP mode support for ls1028a

[Lorenzo Pieralisi]

On Mon, Jan 09, 2023 at 03:41:31PM +, Frank Li wrote:
> > 
> > From: Xiaowei Bao 
> > 
> > Add PCIe EP mode support for ls1028a.
> > 
> > Signed-off-by: Xiaowei Bao 
> > Signed-off-by: Hou Zhiqiang 
> > ---
> > 
> > All other patches were already accepte by maintainer in
> > https://lore.kernel.org/lkml/2022223457.10599-1-leoyang...@nxp.com/
> > 
> > But missed this one.
> > 
> > Re-post.
> > 
> 
> Ping.

You must sign it off since you obviously are in the patch delivery chain:

https://docs.kernel.org/process/submitting-patches.html

> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v7 05/14] sizes.h: Add SZ_1T macro

[Lorenzo Pieralisi]

On Fri, 21 Jan 2022 08:42:21 +, Christophe Leroy wrote:
> Today drivers/pci/controller/pci-xgene.c defines SZ_1T
> 
> Move it into linux/sizes.h so that it can be re-used elsewhere.
> 
> 

Applied to pci/misc, thanks!

[05/14] sizes.h: Add SZ_1T macro
https://git.kernel.org/lpieralisi/pci/c/0cc62aed37

Thanks,
Lorenzo

Re: [PATCH] PCI: layerscape: Correct syntax by changing comma to semicolon

[Lorenzo Pieralisi]

On Thu, 11 Mar 2021 03:37:45 +, Krzysztof Wilczyński wrote:
> Replace command with a semicolon to correct syntax and to prevent
> potential unspecified behaviour and/or unintended side effects.
> 
> Related:
>   
> https://lore.kernel.org/linux-pci/20201216131944.14990-1-zhengyongj...@huawei.com/

Applied to pci/layerscape, thanks!

[1/1] PCI: layerscape: Correct syntax by changing comma to semicolon
  https://git.kernel.org/lpieralisi/pci/c/1b7996a528

Thanks,
Lorenzo

Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon

[Lorenzo Pieralisi]

On Mon, Mar 22, 2021 at 01:40:16PM +, Roy Zang wrote:
> Yes.  It is maintained.

To be maintained you should review its code please.

> I will send out a patch.

Krzysztof already posted one for you, you just need to ack it:

https://patchwork.kernel.org/project/linux-pci/patch/20210311033745.1547044-1...@linux.com

For the future email exchanges: don't top-post please.

Thanks,
Lorenzo

> Thanks.
> Roy
> 
> -Original Message-----
> From: Lorenzo Pieralisi  
> 
> On Sun, Mar 07, 2021 at 07:36:57PM +0100, Krzysztof Wilczyński wrote:
> > Hi,
> > 
> > [...]
> > > I would request NXP maintainers to take this patch, rewrite it as 
> > > Bjorn requested and resend it as fast as possible, this is a very 
> > > relevant fix.
> > [...]
> > 
> > Looking at the state of the pci-layerscape-ep.c file in Linus' tree, 
> > this still hasn't been fixed, and it has been a while.
> > 
> > NXP folks, are you intend to pick this up?  Do let us know.
> 
> Minghuan, Mingkai, Roy,
> 
> either one of you reply and follow up this patch or I will have to update the 
> MAINTAINERS entry and take action accordingly, you are not maintaining this 
> driver and I won't maintain your code, sorry.
> 
> Lorenzo
> 
> > Krzysztof

Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon

[Lorenzo Pieralisi]

On Sun, Mar 07, 2021 at 07:36:57PM +0100, Krzysztof Wilczyński wrote:
> Hi,
> 
> [...]
> > I would request NXP maintainers to take this patch, rewrite it as
> > Bjorn requested and resend it as fast as possible, this is a very
> > relevant fix.
> [...]
> 
> Looking at the state of the pci-layerscape-ep.c file in Linus' tree,
> this still hasn't been fixed, and it has been a while.
> 
> NXP folks, are you intend to pick this up?  Do let us know.

Minghuan, Mingkai, Roy,

either one of you reply and follow up this patch or I will have to
update the MAINTAINERS entry and take action accordingly, you are
not maintaining this driver and I won't maintain your code, sorry.

Lorenzo

> Krzysztof

Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()

[Lorenzo Pieralisi]

On Wed, 20 Jan 2021 11:52:46 +0100, Michael Walle wrote:
> fw_devlink will defer the probe until all suppliers are ready. We can't
> use builtin_platform_driver_probe() because it doesn't retry after probe
> deferral. Convert it to builtin_platform_driver().

Applied to pci/dwc, thanks!

[1/1] PCI: dwc: layerscape: Convert to builtin_platform_driver()
  https://git.kernel.org/lpieralisi/pci/c/538157be1e

Thanks,
Lorenzo

Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()

[Lorenzo Pieralisi]

On Wed, Jan 20, 2021 at 11:52:46AM +0100, Michael Walle wrote:
> fw_devlink will defer the probe until all suppliers are ready. We can't
> use builtin_platform_driver_probe() because it doesn't retry after probe
> deferral. Convert it to builtin_platform_driver().
> 
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")

I will have to drop this Fixes: tag if you don't mind, it is not
in the mainline.

Lorenzo

> Signed-off-by: Michael Walle 
> ---
>  drivers/pci/controller/dwc/pci-layerscape.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c 
> b/drivers/pci/controller/dwc/pci-layerscape.c
> index 44ad34cdc3bc..5b9c625df7b8 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> @@ -232,7 +232,7 @@ static const struct of_device_id ls_pcie_of_match[] = {
>   { },
>  };
>  
> -static int __init ls_pcie_probe(struct platform_device *pdev)
> +static int ls_pcie_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
>   struct dw_pcie *pci;
> @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct platform_device 
> *pdev)
>  }
>  
>  static struct platform_driver ls_pcie_driver = {
> + .probe = ls_pcie_probe,
>   .driver = {
>   .name = "layerscape-pcie",
>   .of_match_table = ls_pcie_of_match,
>   .suppress_bind_attrs = true,
>   },
>  };
> -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe);
> +builtin_platform_driver(ls_pcie_driver);
> -- 
> 2.20.1
> 

Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()

[Lorenzo Pieralisi]

On Wed, Jan 20, 2021 at 08:28:36PM +0100, Michael Walle wrote:
> [RESEND, fat-fingered the buttons of my mail client and converted
> all CCs to BCCs :(]
> 
> Am 2021-01-20 20:02, schrieb Saravana Kannan:
> > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring  wrote:
> > > 
> > > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle 
> > > wrote:
> > > >
> > > > fw_devlink will defer the probe until all suppliers are ready. We can't
> > > > use builtin_platform_driver_probe() because it doesn't retry after probe
> > > > deferral. Convert it to builtin_platform_driver().
> > > 
> > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then
> > > shouldn't it be fixed or removed?
> > 
> > I was actually thinking about this too. The problem with fixing
> > builtin_platform_driver_probe() to behave like
> > builtin_platform_driver() is that these probe functions could be
> > marked with __init. But there are also only 20 instances of
> > builtin_platform_driver_probe() in the kernel:
> > $ git grep ^builtin_platform_driver_probe | wc -l
> > 20
> > 
> > So it might be easier to just fix them to not use
> > builtin_platform_driver_probe().
> > 
> > Michael,
> > 
> > Any chance you'd be willing to help me by converting all these to
> > builtin_platform_driver() and delete builtin_platform_driver_probe()?
> 
> If it just moving the probe function to the _driver struct and
> remove the __init annotations. I could look into that.

Can I drop this patch then ?

Thanks,
Lorenzo

Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon

[Lorenzo Pieralisi]

On Wed, Jan 06, 2021 at 01:07:22PM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 16, 2020 at 09:19:44PM +0800, Zheng Yongjun wrote:
> > Replace a comma between expression statements by a semicolon.
> 
> Looks like a good fix, but read this about the changelog title:
> 
> https://lore.kernel.org/r/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com

I would request NXP maintainers to take this patch, rewrite it as
Bjorn requested and resend it as fast as possible, this is a very
relevant fix.

Thanks,
Lorenzo

> > Signed-off-by: Zheng Yongjun 
> > ---
> >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index 84206f265e54..917ba8d254fc 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -178,7 +178,7 @@ static int __init ls_pcie_ep_probe(struct 
> > platform_device *pdev)
> > pci->dev = dev;
> > pci->ops = pcie->drvdata->dw_pcie_ops;
> >  
> > -   ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > +   ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4);
> >  
> > pcie->pci = pci;
> > pcie->ls_epc = ls_epc;
> > -- 
> > 2.22.0
> > 
> > 
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers

[Lorenzo Pieralisi]

On Sun, 29 Nov 2020 23:07:38 +, Krzysztof Wilczyński wrote:
> Unify ECAM-related constants into a single set of standard constants
> defining memory address shift values for the byte-level address that can
> be used when accessing the PCI Express Configuration Space, and then
> move native PCI Express controller drivers to use newly introduced
> definitions retiring any driver-specific ones.
> 
> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
> PCI Express specification (see PCI Express Base Specification, Revision
> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
> implement it the same way.
> 
> [...]

Applied to pci/ecam, thanks!

[1/5] PCI: Unify ECAM constants in native PCI Express drivers
  https://git.kernel.org/lpieralisi/pci/c/f3c07cf692
[2/5] PCI: thunder-pem: Add constant for custom ".bus_shift" initialiser
  https://git.kernel.org/lpieralisi/pci/c/3c38579263
[3/5] PCI: iproc: Convert to use the new ECAM constants
  https://git.kernel.org/lpieralisi/pci/c/333ec9d3cc
[4/5] PCI: vmd: Update type of the __iomem pointers
  https://git.kernel.org/lpieralisi/pci/c/89094c12ea
[5/5] PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
  https://git.kernel.org/lpieralisi/pci/c/3dc62532a5

Thanks,
Lorenzo

Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers

[Lorenzo Pieralisi]

On Sun, Nov 29, 2020 at 11:07:39PM +, Krzysztof Wilczyński wrote:
> Add ECAM-related constants to provide a set of standard constants
> defining memory address shift values to the byte-level address that can
> be used to access the PCI Express Configuration Space, and then move
> native PCI Express controller drivers to use the newly introduced
> definitions retiring driver-specific ones.
> 
> Refactor pci_ecam_map_bus() function to use newly added constants so
> that limits to the bus, device function and offset (now limited to 4K as
> per the specification) are in place to prevent the defective or
> malicious caller from supplying incorrect configuration offset and thus
> targeting the wrong device when accessing extended configuration space.
> This refactor also allows for the ".bus_shit" initialisers to be dropped
  

Nice typo, I'd fix it while applying it though if you don't mind ;-),
no need to resend it.

Jokes aside, nice piece of work, thanks for that.

> when the user is not using a custom value as a default value will be
> used as per the PCI Express Specification.
> 
> Suggested-by: Bjorn Helgaas 
> Signed-off-by: Krzysztof Wilczyński 

I think Bjorn's reviewed-by still stands so I will apply it.

Thanks !
Lorenzo

> ---
>  drivers/pci/controller/dwc/pcie-al.c| 12 ++---
>  drivers/pci/controller/dwc/pcie-hisi.c  |  2 --
>  drivers/pci/controller/pci-aardvark.c   | 13 +++---
>  drivers/pci/controller/pci-host-generic.c   |  1 -
>  drivers/pci/controller/pci-thunder-ecam.c   |  1 -
>  drivers/pci/controller/pcie-brcmstb.c   | 16 ++--
>  drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
>  drivers/pci/controller/pcie-rockchip.h  |  8 +-
>  drivers/pci/controller/pcie-tango.c |  1 -
>  drivers/pci/controller/pcie-xilinx-nwl.c|  9 ++-
>  drivers/pci/controller/pcie-xilinx.c| 11 ++---
>  drivers/pci/controller/vmd.c| 11 -
>  drivers/pci/ecam.c  | 23 --
>  include/linux/pci-ecam.h| 27 +
>  14 files changed, 73 insertions(+), 89 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-al.c 
> b/drivers/pci/controller/dwc/pcie-al.c
> index f973fbca90cf..af9e51ab1af8 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops al_pcie_ops = {
> - .bus_shift= 20,
>   .init =  al_pcie_init,
>   .pci_ops  = {
>   .map_bus= al_pcie_map_bus,
> @@ -138,8 +137,6 @@ struct al_pcie {
>   struct al_pcie_target_bus_cfg target_bus_cfg;
>  };
>  
> -#define PCIE_ECAM_DEVFN(x)   (((x) & 0xff) << 12)
> -
>  #define to_al_pcie(x)dev_get_drvdata((x)->dev)
>  
>  static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct 
> pci_bus *bus,
>   struct al_pcie_target_bus_cfg *target_bus_cfg = >target_bus_cfg;
>   unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
>   unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
> - void __iomem *pci_base_addr;
> -
> - pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> -  (busnr_ecam << 20) +
> -  PCIE_ECAM_DEVFN(devfn));
>  
>   if (busnr_reg != target_bus_cfg->reg_val) {
>   dev_dbg(pcie->pci->dev, "Changing target bus busnum val from 
> 0x%x to 0x%x\n",
> @@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct 
> pci_bus *bus,
>  target_bus_cfg->reg_mask);
>   }
>  
> - return pci_base_addr + where;
> + return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
>  }
>  
>  static struct pci_ops al_child_pci_ops = {
> @@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>  
>   target_bus_cfg = >target_bus_cfg;
>  
> - ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> + ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
>   if (ecam_bus_mask > 255) {
>   dev_warn(pcie->dev, "ECAM window size is larger than 256MB. 
> Cutting off at 256\n");
>   ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c 
> b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..8fc5960faf28 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
>  }
>  
>  const struct pci_ecam_ops hisi_pcie_ops = {
> - .bus_shift= 20,
>   .init =  hisi_pcie_init,
>   .pci_ops  = {
>   .map_bus  

Re: [PATCH v2 00/16] PCI: dwc: Another round of clean-ups

[Lorenzo Pieralisi]

On Thu, 5 Nov 2020 15:11:43 -0600, Rob Herring wrote:
> Here's another batch of DWC PCI host refactoring. This series primarily
> moves more of the MSI, link up, and resource handling to the core
> code. Beyond a couple of minor fixes, new in this version is runtime
> detection of iATU regions instead of using DT properties.
> 
> No doubt I've probably broken something. Please test. I've run this thru
> kernelci and checked boards with DWC PCI which currently is just
> Layerscape boards (hint: add boards and/or enable PCI). A git branch is
> here[1].
> 
> [...]

Applied to pci/dwc, thanks!

[01/16] PCI: dwc: Support multiple ATU memory regions
https://git.kernel.org/lpieralisi/pci/c/9f9e59a480
[02/16] PCI: dwc/intel-gw: Move ATU offset out of driver match data
https://git.kernel.org/lpieralisi/pci/c/1d567aac46
[03/16] PCI: dwc: Move "dbi", "dbi2", and "addr_space" resource setup into 
common code
https://git.kernel.org/lpieralisi/pci/c/a0fd361db8
[04/16] PCI: dwc/intel-gw: Remove some unneeded function wrappers
https://git.kernel.org/lpieralisi/pci/c/1cc9a55999
[05/16] PCI: dwc: Ensure all outbound ATU windows are reset
https://git.kernel.org/lpieralisi/pci/c/458ad06c4c
[06/16] PCI: dwc/dra7xx: Use the common MSI irq_chip
https://git.kernel.org/lpieralisi/pci/c/7f170d35f5
[07/16] PCI: dwc: Drop the .set_num_vectors() host op
https://git.kernel.org/lpieralisi/pci/c/331e9bcead
[08/16] PCI: dwc: Move MSI interrupt setup into DWC common code
https://git.kernel.org/lpieralisi/pci/c/5bcb1757e6
[09/16] PCI: dwc: Rework MSI initialization
https://git.kernel.org/lpieralisi/pci/c/f78f02638a
[10/16] PCI: dwc: Move link handling into common code
https://git.kernel.org/lpieralisi/pci/c/886a9c1347
[11/16] PCI: dwc: Move dw_pcie_msi_init() into core
https://git.kernel.org/lpieralisi/pci/c/59fbab1ae4
[12/16] PCI: dwc: Move dw_pcie_setup_rc() to DWC common code
https://git.kernel.org/lpieralisi/pci/c/b9ac0f9dc8
[13/16] PCI: dwc: Remove unnecessary wrappers around dw_pcie_host_init()
https://git.kernel.org/lpieralisi/pci/c/60f5b73fa0
[14/16] Revert "PCI: dwc/keystone: Drop duplicated 'num-viewport'"
https://git.kernel.org/lpieralisi/pci/c/fcde397422
[15/16] PCI: dwc: Move inbound and outbound windows to common struct
https://git.kernel.org/lpieralisi/pci/c/9ca17af552
[16/16] PCI: dwc: Detect number of iATU windows
https://git.kernel.org/lpieralisi/pci/c/281f1f99cf

Thanks,
Lorenzo

Re: [PATCHv7 00/12]PCI: dwc: Add the multiple PF support for DWC and Layerscape

[Lorenzo Pieralisi]

On Tue, Aug 11, 2020 at 05:54:29PM +0800, Zhiqiang Hou wrote:
> From: Hou Zhiqiang 
> 
> Add the PCIe EP multiple PF support for DWC and Layerscape, and use
> a list to manage the PFs of each PCIe controller; add the doorbell
> MSIX function for DWC; and refactor the Layerscape EP driver due to
> some difference in Layercape platforms PCIe integration.
> 
> Hou Zhiqiang (1):
>   misc: pci_endpoint_test: Add driver data for Layerscape PCIe
> controllers
> 
> Xiaowei Bao (11):
>   PCI: designware-ep: Add multiple PFs support for DWC
>   PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
>   PCI: designware-ep: Move the function of getting MSI capability
> forward
>   PCI: designware-ep: Modify MSI and MSIX CAP way of finding
>   dt-bindings: pci: layerscape-pci: Add compatible strings for ls1088a
> and ls2088a
>   PCI: layerscape: Fix some format issue of the code
>   PCI: layerscape: Modify the way of getting capability with different
> PEX
>   PCI: layerscape: Modify the MSIX to the doorbell mode
>   PCI: layerscape: Add EP mode support for ls1088a and ls2088a
>   arm64: dts: layerscape: Add PCIe EP node for ls1088a
>   misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> 
>  .../bindings/pci/layerscape-pci.txt   |   2 +
>  .../arm64/boot/dts/freescale/fsl-ls1088a.dtsi |  31 +++
>  drivers/misc/pci_endpoint_test.c  |   8 +-
>  .../pci/controller/dwc/pci-layerscape-ep.c| 100 +--
>  .../pci/controller/dwc/pcie-designware-ep.c   | 258 ++
>  drivers/pci/controller/dwc/pcie-designware.c  |  59 ++--
>  drivers/pci/controller/dwc/pcie-designware.h  |  48 +++-
>  7 files changed, 410 insertions(+), 96 deletions(-)

Side note: I will change it for you but please keep Signed-off-by:
tags together in the log instead of mixing them with other tags
randomly.

Can you rebase this series against my pci/dwc branch please and
send a v8 ?

I will apply it then.

Thanks,
Lorenzo

Re: [PATCH v6 00/11] Add the multiple PF support for DWC and Layerscape

[Lorenzo Pieralisi]

On Sat, Mar 14, 2020 at 11:30:27AM +0800, Xiaowei Bao wrote:
> Add the PCIe EP multiple PF support for DWC and Layerscape, add
> the doorbell MSIX function for DWC, use list to manage the PF of
> one PCIe controller, and refactor the Layerscape EP driver due to
> some platforms difference.
> 
> Xiaowei Bao (11):
>   PCI: designware-ep: Add multiple PFs support for DWC
>   PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
>   PCI: designware-ep: Move the function of getting MSI capability
> forward
>   PCI: designware-ep: Modify MSI and MSIX CAP way of finding
>   dt-bindings: pci: layerscape-pci: Add compatible strings for ls1088a
> and ls2088a
>   PCI: layerscape: Fix some format issue of the code
>   PCI: layerscape: Modify the way of getting capability with different
> PEX
>   PCI: layerscape: Modify the MSIX to the doorbell mode
>   PCI: layerscape: Add EP mode support for ls1088a and ls2088a
>   arm64: dts: layerscape: Add PCIe EP node for ls1088a
>   misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt |   2 +
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |  31 +++
>  drivers/misc/pci_endpoint_test.c   |   2 +
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 100 ++--
>  drivers/pci/controller/dwc/pcie-designware-ep.c| 255 
> +
>  drivers/pci/controller/dwc/pcie-designware.c   |  59 +++--
>  drivers/pci/controller/dwc/pcie-designware.h   |  48 +++-
>  7 files changed, 404 insertions(+), 93 deletions(-)

Can you rebase it against v5.8-rc1 please ?

Thanks,
Lorenzo

Re: [PATCH v4 00/11] Add the multiple PF support for DWC and Layerscape

[Lorenzo Pieralisi]

On Tue, Sep 24, 2019 at 10:18:38AM +0800, Xiaowei Bao wrote:
> Add the PCIe EP multiple PF support for DWC and Layerscape, add
> the doorbell MSIX function for DWC, use list to manage the PF of
> one PCIe controller, and refactor the Layerscape EP driver due to
> some platforms difference.
> 
> Xiaowei Bao (11):
>   PCI: designware-ep: Add multiple PFs support for DWC
>   PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
>   PCI: designware-ep: Move the function of getting MSI capability
> forward
>   PCI: designware-ep: Modify MSI and MSIX CAP way of finding
>   dt-bindings: pci: layerscape-pci: add compatible strings for ls1088a
> and ls2088a
>   PCI: layerscape: Fix some format issue of the code
>   PCI: layerscape: Modify the way of getting capability with different
> PEX
>   PCI: layerscape: Modify the MSIX to the doorbell mode
>   PCI: layerscape: Add EP mode support for ls1088a and ls2088a
>   arm64: dts: layerscape: Add PCIe EP node for ls1088a
>   misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt |   2 +
>  arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi |  31 +++
>  drivers/misc/pci_endpoint_test.c   |   2 +
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 100 ++--
>  drivers/pci/controller/dwc/pcie-designware-ep.c| 255 
> +
>  drivers/pci/controller/dwc/pcie-designware.c   |  59 +++--
>  drivers/pci/controller/dwc/pcie-designware.h   |  48 +++-
>  7 files changed, 404 insertions(+), 93 deletions(-)

Hi,

are you resending this patchset ? I would also like Andrew and Kishon to
have a look and ACK relevant code before merging it.

Thanks,
Lorenzo

Re: [PATCH v6 1/3] dt-bindings: pci: layerscape-pci: add compatible strings "fsl,ls1028a-pcie"

[Lorenzo Pieralisi]

On Mon, Sep 02, 2019 at 11:43:17AM +0800, Xiaowei Bao wrote:
> Add the PCIe compatible string for LS1028A

Sentences must be terminated with a period.

> Signed-off-by: Xiaowei Bao 
> Signed-off-by: Hou Zhiqiang 
> Reviewed-by: Rob Herring 
> ---
> v2:
>  - No change.
> v3:
>  - No change.
> v4:
>  - No change.
> v5:
>  - No change.
> v6:
>  - No change.
> 
>  Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt 
> b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index e20ceaa..99a386e 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -21,6 +21,7 @@ Required properties:
>  "fsl,ls1046a-pcie"
>  "fsl,ls1043a-pcie"
>  "fsl,ls1012a-pcie"
> +"fsl,ls1028a-pcie"
>EP mode:
>   "fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
>  - reg: base addresses and lengths of the PCIe controller register blocks.

I have applied this series to pci/layerscape, thanks.

Lorenzo

Re: [PATCH v6 3/3] PCI: layerscape: Add LS1028a support

[Lorenzo Pieralisi]

On Wed, Nov 06, 2019 at 03:46:17AM +, M.h. Lian wrote:
> Hi Lorenzo,
> 
> Sorry for the late reply.
> 
> Acked-by: Minghuan Lian 

https://en.wikipedia.org/wiki/Posting_style#Top-posting

Never top-post on kernel mailing lists.

Thanks,
Lorenzo

> Thanks,
> Minghuan
> 
> > -Original Message-
> > From: Xiaowei Bao 
> > Sent: Wednesday, November 6, 2019 11:36 AM
> > To: Lorenzo Pieralisi 
> > Cc: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org;
> > Leo Li ; M.h. Lian ; Mingkai
> > Hu ; Roy Zang ; linux-
> > p...@vger.kernel.org; devicet...@vger.kernel.org; linux-
> > ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linuxppc-
> > d...@lists.ozlabs.org; bhelg...@google.com; Z.q. Hou
> > 
> > Subject: RE: [PATCH v6 3/3] PCI: layerscape: Add LS1028a support
> > 
> > 
> > 
> > > -Original Message-
> > > From: Lorenzo Pieralisi 
> > > Sent: 2019年11月5日 20:33
> > > To: Xiaowei Bao 
> > > Cc: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org;
> > Leo
> > > Li ; 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; bhelg...@google.com; Z.q. Hou
> > > 
> > > Subject: Re: [PATCH v6 3/3] PCI: layerscape: Add LS1028a support
> > >
> > > On Mon, Sep 02, 2019 at 11:43:19AM +0800, Xiaowei Bao wrote:
> > > > Add support for the LS1028a PCIe controller.
> > > >
> > > > Signed-off-by: Xiaowei Bao 
> > > > Signed-off-by: Hou Zhiqiang 
> > > > ---
> > > > v2:
> > > >  - No change.
> > > > v3:
> > > >  - Reuse the ls2088 driver data structurt.
> > > > v4:
> > > >  - No change.
> > > > v5:
> > > >  - No change.
> > > > v6:
> > > >  - No change.
> > > >
> > > >  drivers/pci/controller/dwc/pci-layerscape.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > >
> > > I have not seen any comment on any layerscape driver patches coming
> > > from the maintainers as listed in the MAINTAINERS file (and CCed in this
> > series).
> > >
> > > I request maintainers ACK on these patches and I expect them to start
> > > reviewing your code if they want to be still considered maintainers
> > > for this driver.
> > >
> > > The changes look OK minus Shawn's remark on compatible string that was
> > > ignored.
> > 
> > Hi Lorenzo,
> > 
> > Thanks for your comments.
> > 
> > In fact, the patches have reviewed in our internal mail list, after the 
> > review by
> > Minghuan and Mingkai, I will send these patches to opensource, they will
> > give the ACK when these patches seems is OK and no comments on this.
> > 
> > Thanks
> > Xiaowei
> > 
> > >
> > > Thanks,
> > > Lorenzo
> > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape.c
> > > > b/drivers/pci/controller/dwc/pci-layerscape.c
> > > > index 3a5fa26..f24f79a 100644
> > > > --- a/drivers/pci/controller/dwc/pci-layerscape.c
> > > > +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> > > > @@ -263,6 +263,7 @@ static const struct ls_pcie_drvdata
> > > > ls2088_drvdata = {  static const struct of_device_id ls_pcie_of_match[] 
> > > > = {
> > > > { .compatible = "fsl,ls1012a-pcie", .data = _drvdata },
> > > > { .compatible = "fsl,ls1021a-pcie", .data = _drvdata },
> > > > +   { .compatible = "fsl,ls1028a-pcie", .data = _drvdata },
> > > > { .compatible = "fsl,ls1043a-pcie", .data = _drvdata },
> > > > { .compatible = "fsl,ls1046a-pcie", .data = _drvdata },
> > > > { .compatible = "fsl,ls2080a-pcie", .data = _drvdata },
> > > > --
> > > > 2.9.5
> > > >

Re: [PATCH v2 07/10] PCI: layerscape: Modify the MSIX to the doorbell way

[Lorenzo Pieralisi]

On Thu, Aug 29, 2019 at 10:43:18AM +0530, Kishon Vijay Abraham I wrote:
> Gustavo,
> 
> On 27/08/19 6:55 PM, Andrew Murray wrote:
> > On Sat, Aug 24, 2019 at 12:08:40AM +, Xiaowei Bao wrote:
> >>
> >>
> >>> -Original Message-
> >>> From: Andrew Murray 
> >>> Sent: 2019年8月23日 21:58
> >>> 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 07/10] PCI: layerscape: Modify the MSIX to the
> >>> doorbell way
> >>>
> >>> On Thu, Aug 22, 2019 at 07:22:39PM +0800, Xiaowei Bao wrote:
>  The layerscape platform use the doorbell way to trigger MSIX interrupt
>  in EP mode.
> 
> >>>
> >>> I have no problems with this patch, however...
> >>>
> >>> Are you able to add to this message a reason for why you are making this
> >>> change? Did dw_pcie_ep_raise_msix_irq not work when func_no != 0? Or did
> >>> it work yet dw_pcie_ep_raise_msix_irq_doorbell is more efficient?
> >>
> >> The fact is that, this driver is verified in ls1046a platform of NXP 
> >> before, and ls1046a don't
> >> support MSIX feature, so I set the msix_capable of pci_epc_features struct 
> >> is false,
> >> but in other platform, e.g. ls1088a, it support the MSIX feature, I 
> >> verified the MSIX
> >> feature in ls1088a, it is not OK, so I changed to another way. Thanks.
> > 
> > Right, so the existing pci-layerscape-ep.c driver never supported MSIX yet 
> > it
> > erroneously had a switch case statement to call dw_pcie_ep_raise_msix_irq 
> > which
> > would never get used.
> > 
> > Now that we're adding a platform with MSIX support the existing
> > dw_pcie_ep_raise_msix_irq doesn't work (for this platform) so we are adding 
> > a
> > different method.
> 
> Gustavo, can you confirm dw_pcie_ep_raise_msix_irq() works for
> designware as it didn't work for both me and Xiaowei?

This question needs an answer.

Thanks,
Lorenzo

Re: [PATCH v6 3/3] PCI: layerscape: Add LS1028a support

[Lorenzo Pieralisi]

On Mon, Sep 02, 2019 at 11:43:19AM +0800, Xiaowei Bao wrote:
> Add support for the LS1028a PCIe controller.
> 
> Signed-off-by: Xiaowei Bao 
> Signed-off-by: Hou Zhiqiang 
> ---
> v2:
>  - No change.
> v3:
>  - Reuse the ls2088 driver data structurt.
> v4:
>  - No change.
> v5:
>  - No change.
> v6:
>  - No change.
> 
>  drivers/pci/controller/dwc/pci-layerscape.c | 1 +
>  1 file changed, 1 insertion(+)

I have not seen any comment on any layerscape driver patches
coming from the maintainers as listed in the MAINTAINERS
file (and CCed in this series).

I request maintainers ACK on these patches and I expect them
to start reviewing your code if they want to be still considered
maintainers for this driver.

The changes look OK minus Shawn's remark on compatible string
that was ignored.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c 
> b/drivers/pci/controller/dwc/pci-layerscape.c
> index 3a5fa26..f24f79a 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> @@ -263,6 +263,7 @@ static const struct ls_pcie_drvdata ls2088_drvdata = {
>  static const struct of_device_id ls_pcie_of_match[] = {
>   { .compatible = "fsl,ls1012a-pcie", .data = _drvdata },
>   { .compatible = "fsl,ls1021a-pcie", .data = _drvdata },
> + { .compatible = "fsl,ls1028a-pcie", .data = _drvdata },
>   { .compatible = "fsl,ls1043a-pcie", .data = _drvdata },
>   { .compatible = "fsl,ls1046a-pcie", .data = _drvdata },
>   { .compatible = "fsl,ls2080a-pcie", .data = _drvdata },
> -- 
> 2.9.5
> 

Re: [PATCH v4 1/3] dt-bindings: pci: layerscape-pci: add compatible strings "fsl,ls1028a-pcie"

[Lorenzo Pieralisi]

On Fri, Aug 23, 2019 at 04:26:41PM +0800, Xiaowei Bao wrote:
> Add the PCIe compatible string for LS1028A
> 
> Signed-off-by: Xiaowei Bao 
> Signed-off-by: Hou Zhiqiang 
> Reviewed-by: Rob Herring 
> ---
> v2:
>  - No change.
> v3:
>  - No change.
> v4:
>  - No change.
> 
>  Documentation/devicetree/bindings/pci/layerscape-pci.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt 
> b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index e20ceaa..99a386e 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -21,6 +21,7 @@ Required properties:
>  "fsl,ls1046a-pcie"
>  "fsl,ls1043a-pcie"
>  "fsl,ls1012a-pcie"
> +"fsl,ls1028a-pcie"
>EP mode:
>   "fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
>  - reg: base addresses and lengths of the PCIe controller register blocks.

This series does not apply to v5.3-rc1, what is it based on ?

Lorenzo

Re: [PATCHv6 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.

[Lorenzo Pieralisi]

On Wed, Aug 14, 2019 at 09:48:00AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Lorenzo Pieralisi 
> > Sent: 2019年8月14日 17:30
> > To: Xiaowei Bao 
> > Cc: M.h. Lian ; Mingkai Hu
> > ; Roy Zang ;
> > bhelg...@google.com; linuxppc-dev@lists.ozlabs.org;
> > linux-...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> > linux-ker...@vger.kernel.org
> > Subject: Re: [PATCHv6 1/2] PCI: layerscape: Add the bar_fixed_64bit property
> > in EP driver.

Do not quote the email header in your replies.

> > I asked you to remove the period at the end of the patch $SUBJECT and you
> > did not, either you do not read what I write or explain me what's going on.
> Sorry, I didn't understand the meaning of period correctly before. 
> > 
> > On Wed, Aug 14, 2019 at 10:03:29AM +0800, Xiaowei Bao wrote:
> > > The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1 is
> > > 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware, so set
> > > the bar_fixed_64bit with 0x14.
> > >
> > > Signed-off-by: Xiaowei Bao 
> > 
> > Kishon ACK'ed this patch and you have not carried his tag.
> > 
> > I will make these changes but that's the last time I do that for you.
> Thanks a lot, your means is that I don't need to send the v7 patch and you 
> help me to
> Correct this patch, yes? Thanks a lot for your help about the rules of the 
> upstream. I will
> Correct this error next time. ^.^ 

I fixed that up and pushed out, pci/layerscape, for v5.4.

Thanks,
Lorenzo

> > Lorenzo
> > 
> > > ---
> > > v2:
> > >  - Replace value 0x14 with a macro.
> > > v3:
> > >  - No change.
> > > v4:
> > >  - send the patch again with '--to'.
> > > v5:
> > >  - fix the commit message.
> > > v6:
> > >  - remove the [EXT] tag of the $SUBJECT in email.
> > >
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index be61d96..ca9aa45 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -44,6 +44,7 @@ static const struct pci_epc_features
> > ls_pcie_epc_features = {
> > >   .linkup_notifier = false,
> > >   .msi_capable = true,
> > >   .msix_capable = false,
> > > + .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > >  };
> > >
> > >  static const struct pci_epc_features*
> > > --
> > > 2.9.5
> > >

Re: [PATCHv6 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.

[Lorenzo Pieralisi]

I asked you to remove the period at the end of the patch $SUBJECT and
you did not, either you do not read what I write or explain me what's
going on.

On Wed, Aug 14, 2019 at 10:03:29AM +0800, Xiaowei Bao wrote:
> The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1
> is 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware,
> so set the bar_fixed_64bit with 0x14.
> 
> Signed-off-by: Xiaowei Bao 

Kishon ACK'ed this patch and you have not carried his tag.

I will make these changes but that's the last time I do that
for you.

Lorenzo

> ---
> v2:
>  - Replace value 0x14 with a macro.
> v3:
>  - No change.
> v4:
>  - send the patch again with '--to'.
> v5:
>  - fix the commit message.
> v6:
>  - remove the [EXT] tag of the $SUBJECT in email.
> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index be61d96..ca9aa45 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -44,6 +44,7 @@ static const struct pci_epc_features ls_pcie_epc_features = 
> {
>   .linkup_notifier = false,
>   .msi_capable = true,
>   .msix_capable = false,
> + .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
>  };
>  
>  static const struct pci_epc_features*
> -- 
> 2.9.5
> 

Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.

[Lorenzo Pieralisi]

git log --oneline --follow drivers/pci/controller/dwc/pci-layerscape.c

Do you see any commit with a $SUBJECT ending with a period ?

There is not. So remove it from yours too.

On Tue, Aug 13, 2019 at 02:28:39PM +0800, Xiaowei Bao wrote:
> The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1
> is 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware,
> so set the bar_fixed_64bit with 0x14.
> 
> Signed-off-by: Xiaowei Bao 
> ---
> v2:
>  - Replace value 0x14 with a macro.
> v3:
>  - No change.
> v4:
>  - send the patch again with '--to'.
> v5:
>  - fix the commit message.
> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
>  1 file changed, 1 insertion(+)

scripts/get_maintainer.pl -f drivers/pci/controller/dwc/pci-layerscape-ep.c
Now, with the output you get justify all the people you send this email
to.

So, again, trim the CC list and it is the last time I tell you.

Before sending patches on mailing lists use git --dry-run to check
the emails you are sending.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index be61d96..ca9aa45 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -44,6 +44,7 @@ static const struct pci_epc_features ls_pcie_epc_features = 
> {
>   .linkup_notifier = false,
>   .msi_capable = true,
>   .msix_capable = false,
> + .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
>  };
>  
>  static const struct pci_epc_features*
> -- 
> 2.9.5
> 

Re: [EXT] Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.

[Lorenzo Pieralisi]

You should fix your email client set-up to avoid sticking an [EXT]
tag to your emails $SUBJECT.

On Tue, Aug 13, 2019 at 07:39:48AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Kishon Vijay Abraham I 
> > Sent: 2019年8月13日 15:30
> > To: Xiaowei Bao ; lorenzo.pieral...@arm.com;
> > bhelg...@google.com; M.h. Lian ; Mingkai Hu
> > ; Roy Zang ;
> > l.st...@pengutronix.de; tpie...@impinj.com; Leonard Crestez
> > ; andrew.smir...@gmail.com;
> > yue.w...@amlogic.com; hayashi.kunih...@socionext.com;
> > d...@amazon.co.uk; jon...@amazon.com; linux-...@vger.kernel.org;
> > linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
> > linux-arm-ker...@lists.infradead.org
> > Subject: [EXT] Re: [PATCHv5 1/2] PCI: layerscape: Add the bar_fixed_64bit
> > property in EP driver.
> > 
> > Caution: EXT Email

See above, this "Caution" stuff should disappear.

Also, quoting the email header is useless, please configure your email
client to remove it.

Thanks,
Lorenzo

> > On 13/08/19 11:58 AM, Xiaowei Bao wrote:
> > > The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1 is
> > > 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware, so set
> > > the bar_fixed_64bit with 0x14.
> > >
> > > Signed-off-by: Xiaowei Bao 
> > 
> > Acked-by: Kishon Vijay Abraham I 
> > > ---
> > > v2:
> > >  - Replace value 0x14 with a macro.
> > > v3:
> > >  - No change.
> > > v4:
> > >  - send the patch again with '--to'.
> > > v5:
> > >  - fix the commit message.
> > >
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index be61d96..ca9aa45 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -44,6 +44,7 @@ static const struct pci_epc_features
> > ls_pcie_epc_features = {
> > >   .linkup_notifier = false,
> > >   .msi_capable = true,
> > >   .msix_capable = false,
> > > + .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > >  };
> > >
> > >  static const struct pci_epc_features*
> I check other platforms, it is 'static const struct pci_epc_features', I can 
> get the correct 
> Value use this define way in pci-epf-test.c file.
> > >

Re: [EXT] Re: [PATCHv3 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.

[Lorenzo Pieralisi]

On Mon, Aug 12, 2019 at 10:39:00AM +, Xiaowei Bao wrote:
> 
> 
> > -Original Message-
> > From: Lorenzo Pieralisi 
> > Sent: 2019年8月12日 18:12
> > To: Xiaowei Bao ; kis...@ti.com
> > Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> > shawn...@kernel.org; Leo Li ; a...@arndb.de;
> > gre...@linuxfoundation.org; M.h. Lian ; Mingkai
> > Hu ; Roy Zang ;
> > kstew...@linuxfoundation.org; pombreda...@nexb.com;
> > shawn@rock-chips.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: [EXT] Re: [PATCHv3 1/2] PCI: layerscape: Add the bar_fixed_64bit
> > property in EP driver.
> > 
> > Caution: EXT Email
> > 
> > First off:
> > 
> > Trim the CC list, you CC'ed maintainers (and mailing lists) for no reasons
> > whatsover.
> [Xiaowei Bao]Hi Lorenzo, I am not clear why the mail list is the CC, I use 
> the command "git send-email --to", I will try to send the patch again, do I 
> need to modify the version is v4 when I send this patch again?

Yes you do.

Wrap lines to max 80 characters. There is no need to add [Xiaowei Bao].

1) Read, email etiquette

https://kernelnewbies.org/PatchCulture

2) get_maintainer.pl -f drivers/pci/controller/dwc/pci-layerscape.c

Compare the output to the people in CC, trim it accordingly.

3) The NXP maintainers in the MAINTAINERS file have not given a single
   comment for this patchset. Either they show up or I will remove them
   from the MAINTAINERS list.

4) Before submitting patches, talk to someone at NXP who can help you
   format them in preparation for posting, I do not have time to write
   guidelines for everyone posting on linux-pci, sorry, the information
   is out there if you care to read it.

Thanks,
Lorenzo

> > 
> > Then, read this:
> > 
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> > rnel.org%2Flinux-pci%2F20171026223701.GA25649%40bhelgaas-glaptop.roa
> > m.corp.google.com%2Fdata=02%7C01%7Cxiaowei.bao%40nxp.com%7
> > C1c586178e23c423a0e8808d71f0d8f6f%7C686ea1d3bc2b4c6fa92cd99c5c30
> > 1635%7C0%7C0%7C637012015426788575sdata=3bx1bDFIzik8FnD0wl
> > duAUv7wtLdD1J3hQ3xNH2xmFY%3Dreserved=0
> > 
> > and make your patches compliant please.
> > 
> > On Fri, Jun 28, 2019 at 09:38:25AM +0800, Xiaowei Bao wrote:
> > > The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1 is
> > > 32bit, BAR3 and BAR4 is 64bit, this is determined by hardware, so set
> > > the bar_fixed_64bit with 0x14.
> > >
> > > Signed-off-by: Xiaowei Bao 
> > > ---
> > > v2:
> > >  - Replace value 0x14 with a macro.
> > > v3:
> > >  - No change.
> > >
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c |1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index be61d96..227c33b 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -44,6 +44,7 @@ static int ls_pcie_establish_link(struct dw_pcie *pci)
> > >   .linkup_notifier = false,
> > >   .msi_capable = true,
> > >   .msix_capable = false,
> > > + .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > 
> > I would appreciate Kishon's ACK on this.
> > 
> > Lorenzo
> > 
> > >  };
> > >
> > >  static const struct pci_epc_features*
> > > --
> > > 1.7.1
> > >

Re: [PATCHv3 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.

[Lorenzo Pieralisi]

First off:

Trim the CC list, you CC'ed maintainers (and mailing lists) for no
reasons whatsover.

Then, read this:

https://lore.kernel.org/linux-pci/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com/

and make your patches compliant please.

On Fri, Jun 28, 2019 at 09:38:25AM +0800, Xiaowei Bao wrote:
> The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1
> is 32bit, BAR3 and BAR4 is 64bit, this is determined by hardware,
> so set the bar_fixed_64bit with 0x14.
> 
> Signed-off-by: Xiaowei Bao 
> ---
> v2:
>  - Replace value 0x14 with a macro.
> v3:
>  - No change.
> 
>  drivers/pci/controller/dwc/pci-layerscape-ep.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index be61d96..227c33b 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -44,6 +44,7 @@ static int ls_pcie_establish_link(struct dw_pcie *pci)
>   .linkup_notifier = false,
>   .msi_capable = true,
>   .msix_capable = false,
> + .bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),

I would appreciate Kishon's ACK on this.

Lorenzo

>  };
>  
>  static const struct pci_epc_features*
> -- 
> 1.7.1
> 

Re: [PATCHv3 2/2] PCI: layerscape: Add CONFIG_PCI_LAYERSCAPE_EP to build EP/RC separately

[Lorenzo Pieralisi]

On Fri, Jun 28, 2019 at 09:38:26AM +0800, Xiaowei Bao wrote:
> Add CONFIG_PCI_LAYERSCAPE_EP to build EP/RC separately.
> 
> Signed-off-by: Xiaowei Bao 
> ---
> v2:
>  - No change.
> v3:
>  - modify the commit message.
> 
>  drivers/pci/controller/dwc/Kconfig  |   20 ++--
>  drivers/pci/controller/dwc/Makefile |3 ++-
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig 
> b/drivers/pci/controller/dwc/Kconfig
> index a6ce1ee..a41ccf5 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -131,13 +131,29 @@ config PCI_KEYSTONE_EP
> DesignWare core functions to implement the driver.
>  
>  config PCI_LAYERSCAPE
> - bool "Freescale Layerscape PCIe controller"
> + bool "Freescale Layerscape PCIe controller - Host mode"
>   depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
>   depends on PCI_MSI_IRQ_DOMAIN
>   select MFD_SYSCON
>   select PCIE_DW_HOST
>   help
> -   Say Y here if you want PCIe controller support on Layerscape SoCs.
> +   Say Y here if you want to enable PCIe controller support on Layerscape
> +   SoCs to work in Host mode.
> +   This controller can work either as EP or RC. The RCW[HOST_AGT_PEX]

What's "The RCW" ? This entry should explain why a kernel configuration
should enable it.

Lorenzo

> +   determines which PCIe controller works in EP mode and which PCIe
> +   controller works in RC mode.
> +
> +config PCI_LAYERSCAPE_EP
> + bool "Freescale Layerscape PCIe controller - Endpoint mode"
> + depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
> + depends on PCI_ENDPOINT
> + select PCIE_DW_EP
> + help
> +   Say Y here if you want to enable PCIe controller support on Layerscape
> +   SoCs to work in Endpoint mode.
> +   This controller can work either as EP or RC. The RCW[HOST_AGT_PEX]
> +   determines which PCIe controller works in EP mode and which PCIe
> +   controller works in RC mode.
>  
>  config PCI_HISI
>   depends on OF && (ARM64 || COMPILE_TEST)
> diff --git a/drivers/pci/controller/dwc/Makefile 
> b/drivers/pci/controller/dwc/Makefile
> index b085dfd..824fde7 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,8 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o
>  obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> -- 
> 1.7.1
> 

Re: [PATCH 1/8] PCI: dwc: pci-dra7xx: fix a leaked reference by adding missing of_node_put

[Lorenzo Pieralisi]

On Wed, Feb 27, 2019 at 12:40:36PM +0800, Wen Yang wrote:
> The call to of_get_next_child returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
> irq_domain_add_linear also calls of_node_get to increase refcount,
> so irq_domain will not be affected when it is released.
> 
> Detected by coccinelle with the following warnings:
> ./drivers/pci/controller/dwc/pci-dra7xx.c:252:2-8: ERROR: missing 
> of_node_put; acquired a node pointer with refcount incremented on line 241, 
> but without a corresponding object release within this function.
> ./drivers/pci/controller/dwc/pci-dra7xx.c:255:1-7: ERROR: missing 
> of_node_put; acquired a node pointer with refcount incremented on line 241, 
> but without a corresponding object release within this function.
> 
> Signed-off-by: Wen Yang 
> Cc: Kishon Vijay Abraham I 
> Cc: Lorenzo Pieralisi 
> Cc: Bjorn Helgaas 
> Cc: linux-o...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> ---
>  drivers/pci/controller/dwc/pci-dra7xx.c | 1 +
>  1 file changed, 1 insertion(+)

Applied the series to pci/misc for v5.2, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c 
> b/drivers/pci/controller/dwc/pci-dra7xx.c
> index ae84a69..627c91d 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -247,6 +247,7 @@ static int dra7xx_pcie_init_irq_domain(struct pcie_port 
> *pp)
>  
>   dra7xx->irq_domain = irq_domain_add_linear(pcie_intc_node, PCI_NUM_INTX,
>  _domain_ops, pp);
> + of_node_put(pcie_intc_node);
>   if (!dra7xx->irq_domain) {
>   dev_err(dev, "Failed to get a INTx IRQ domain\n");
>   return -ENODEV;
> -- 
> 2.9.5
> 

Re: [PATCHv6 3/4] pci: layerscape: Add the EP mode support.

[Lorenzo Pieralisi]

On Wed, Feb 20, 2019 at 03:09:01AM +, Xiaowei Bao wrote:
> 
> 
> -Original Message-
> From: Lorenzo Pieralisi  
> Sent: 2019年2月19日 19:27
> To: Xiaowei Bao 
> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com; 
> shawn...@kernel.org; Leo Li ; kis...@ti.com; 
> a...@arndb.de; gre...@linuxfoundation.org; M.h. Lian ; 
> Mingkai Hu ; Roy Zang ; 
> kstew...@linuxfoundation.org; cyrille.pitc...@free-electrons.com; 
> pombreda...@nexb.com; shawn@rock-chips.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: [PATCHv6 3/4] pci: layerscape: Add the EP mode support.
> 
> On Tue, Jan 22, 2019 at 02:33:27PM +0800, Xiaowei Bao wrote:
> > Add the PCIe EP mode support for layerscape platform.
> > 
> > Signed-off-by: Xiaowei Bao 
> > Reviewed-by: Minghuan Lian 
> > Reviewed-by: Zhiqiang Hou 
> > Reviewed-by: Kishon Vijay Abraham I 
> > ---
> > depends on: 
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> > chwork.kernel.org%2Fproject%2Flinux-pci%2Flist%2F%3Fseries%3D66177
> > ;data=02%7C01%7Cxiaowei.bao%40nxp.com%7C6f8772ba47c74d8ee0aa08d6965d3e
> > b4%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636861724572611193
> > ;sdata=b3Acj0fu7c9vSoHe9VzeAkEMbMkpyfYPsXtf6fA8Flk%3Dreserved=0
> > 
> > v2:
> >  - remove the EP mode check function.
> > v3:
> >  - modif the return value when enter default case.
> > v4:
> >  - no change.
> > v5:
> >  - no change.
> > v6:
> >  - modify the code base on the submit patch of the EP framework.
> 
> Can I apply this series to my pci/endpoint branch (where I queued Kishon's EP 
> features rework patches) ? Can you check please ?
> [Xiaowei Bao] of course, in my patch, I found a compile warning, but
> this series patch have approved by you, I don't know how to do, the
> compile warning: " struct pci_epc *epc = ep->epc;" in
> "ls_pcie_ep_init" function, I am so sorry, could you help me remove
> this code, thanks a lot.

If you want me to apply your patches you need to rebase them against
my pci/endpoint branch and make sure the code is correct, I have
applied your previous series but as you know it failed because it
depends on Kishon's clean-up series.

So rebase your code against my pci/endpoint branch, make sure it
compiles with no warnings, test it and send a v7.

Thanks,
Lorenzo

> Thanks,
> Lorenzo
> 
> >  drivers/pci/controller/dwc/Makefile|2 +-
> >  drivers/pci/controller/dwc/pci-layerscape-ep.c |  157 
> > 
> >  2 files changed, 158 insertions(+), 1 deletions(-)  create mode 
> > 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
> > 
> > diff --git a/drivers/pci/controller/dwc/Makefile 
> > b/drivers/pci/controller/dwc/Makefile
> > index 7bcdcdf..b5f3b83 100644
> > --- a/drivers/pci/controller/dwc/Makefile
> > +++ b/drivers/pci/controller/dwc/Makefile
> > @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> >  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> >  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> >  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> > -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> > +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> >  obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> >  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> >  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git 
> > a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > new file mode 100644
> > index 000..ddc2dbb
> > --- /dev/null
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -0,0 +1,157 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * PCIe controller EP driver for Freescale Layerscape SoCs
> > + *
> > + * Copyright (C) 2018 NXP Semiconductor.
> > + *
> > + * Author: Xiaowei Bao   */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "pcie-designware.h"
> > +
> > +#define PCIE_DBI2_OFFSET   0x1000  /* DBI2 base address*/
> > +
> > +struct ls_pcie_ep {
> > +   struct dw_pcie  *pci;
> > +};
> > +
> > +#define to_ls_pcie_ep(x)   dev_get_drvdata((x)->dev)
> > +
> > +static int ls_pcie_establish_link(struct dw_pcie *pci) {
> > +   return 0;
> > +}
> &

Re: [PATCHv6 3/4] pci: layerscape: Add the EP mode support.

[Lorenzo Pieralisi]

On Tue, Jan 22, 2019 at 02:33:27PM +0800, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
> 
> Signed-off-by: Xiaowei Bao 
> Reviewed-by: Minghuan Lian 
> Reviewed-by: Zhiqiang Hou 
> Reviewed-by: Kishon Vijay Abraham I 
> ---
> depends on: https://patchwork.kernel.org/project/linux-pci/list/?series=66177
> 
> v2:
>  - remove the EP mode check function.
> v3:
>  - modif the return value when enter default case.
> v4:
>  - no change.
> v5:
>  - no change.
> v6:
>  - modify the code base on the submit patch of the EP framework.

Can I apply this series to my pci/endpoint branch (where I queued
Kishon's EP features rework patches) ? Can you check please ?

Thanks,
Lorenzo

>  drivers/pci/controller/dwc/Makefile|2 +-
>  drivers/pci/controller/dwc/pci-layerscape-ep.c |  157 
> 
>  2 files changed, 158 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
> 
> diff --git a/drivers/pci/controller/dwc/Makefile 
> b/drivers/pci/controller/dwc/Makefile
> index 7bcdcdf..b5f3b83 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
>  obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> new file mode 100644
> index 000..ddc2dbb
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,157 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_DBI2_OFFSET 0x1000  /* DBI2 base address*/
> +
> +struct ls_pcie_ep {
> + struct dw_pcie  *pci;
> +};
> +
> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> +
> +static int ls_pcie_establish_link(struct dw_pcie *pci)
> +{
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops 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_epc_features = {
> + .linkup_notifier = false,
> + .msi_capable = true,
> + .msix_capable = false,
> +};
> +
> +static const struct pci_epc_features*
> +ls_pcie_ep_get_features(struct dw_pcie_ep *ep)
> +{
> + return _pcie_epc_features;
> +}
> +
> +static void ls_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> + enum pci_barno bar;
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +}
> +
> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +   enum pci_epc_irq_type type, u16 interrupt_num)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_EPC_IRQ_LEGACY:
> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> + case PCI_EPC_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + case PCI_EPC_IRQ_MSIX:
> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + return -EINVAL;
> + }
> +}
> +
> +static struct dw_pcie_ep_ops pcie_ep_ops = {
> + .ep_init = ls_pcie_ep_init,
> + .raise_irq = ls_pcie_ep_raise_irq,
> + .get_features = ls_pcie_ep_get_features,
> +};
> +
> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_pcie_ep *ep;
> + struct resource *res;
> + int ret;
> +
> + ep = >ep;
> + ep->ops = _ep_ops;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> + if (!res)
> + return -EINVAL;
> +
> + ep->phys_base = res->start;
> + ep->addr_size = resource_size(res);
> +
> + ret = dw_pcie_ep_init(ep);
> + if (ret) {
> + dev_err(dev, "failed to initialize endpoint\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> 

Re: [PATCHv6 1/4] dt-bindings: add DT binding for the layerscape PCIe controller with EP mode

[Lorenzo Pieralisi]

On Tue, Jan 22, 2019 at 02:33:25PM +0800, Xiaowei Bao wrote:
> Add the documentation for the Device Tree binding for the layerscape PCIe
> controller with EP mode.
> 
> Signed-off-by: Xiaowei Bao 
> Reviewed-by: Minghuan Lian 
> Reviewed-by: Zhiqiang Hou 
> Reviewed-by: Rob Herring 
> ---
> v2:
>  - Add the SoC specific compatibles.
> v3:
>  - modify the commit message.
> v4:
>  - no change.
> v5:
>  - no change.
> v6:
>  - no change.
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Applied the series to pci/layerscape for v5.1, thanks.

Lorenzo

> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt 
> b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 9b2b8d6..e20ceaa 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -13,6 +13,7 @@ information.
>  
>  Required properties:
>  - compatible: should contain the platform identifier such as:
> +  RC mode:
>  "fsl,ls1021a-pcie"
>  "fsl,ls2080a-pcie", "fsl,ls2085a-pcie"
>  "fsl,ls2088a-pcie"
> @@ -20,6 +21,8 @@ Required properties:
>  "fsl,ls1046a-pcie"
>  "fsl,ls1043a-pcie"
>  "fsl,ls1012a-pcie"
> +  EP mode:
> + "fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
>  - reg: base addresses and lengths of the PCIe controller register blocks.
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>entry for each entry in the interrupt-names property.
> -- 
> 1.7.1
> 

Re: [PATCHv5 1/4] dt-bindings: add DT binding for the layerscape PCIe controller with EP mode

[Lorenzo Pieralisi]

Rob,

Is it OK for you if I pull this series into the pci tree ?

Please let me know, thanks.

Lorenzo

On Mon, Jan 21, 2019 at 05:44:57PM +0800, Xiaowei Bao wrote:
> Add the documentation for the Device Tree binding for the layerscape PCIe
> controller with EP mode.
> 
> Signed-off-by: Xiaowei Bao 
> Reviewed-by: Minghuan Lian 
> Reviewed-by: Zhiqiang Hou 
> ---
> v2:
>  - Add the SoC specific compatibles.
> v3:
>  - modify the commit message.
> v4:
>  - no change.
> v5:
>  - no change.
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt 
> b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 9b2b8d6..e20ceaa 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -13,6 +13,7 @@ information.
>  
>  Required properties:
>  - compatible: should contain the platform identifier such as:
> +  RC mode:
>  "fsl,ls1021a-pcie"
>  "fsl,ls2080a-pcie", "fsl,ls2085a-pcie"
>  "fsl,ls2088a-pcie"
> @@ -20,6 +21,8 @@ Required properties:
>  "fsl,ls1046a-pcie"
>  "fsl,ls1043a-pcie"
>  "fsl,ls1012a-pcie"
> +  EP mode:
> + "fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
>  - reg: base addresses and lengths of the PCIe controller register blocks.
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>entry for each entry in the interrupt-names property.
> -- 
> 1.7.1
> 

Re: [PATCHv2 5/6] pci: layerscape: Add the EP mode support.

[Lorenzo Pieralisi]

On Mon, Nov 05, 2018 at 04:46:52PM +0800, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
> 
> Signed-off-by: Xiaowei Bao 
> ---
> v2:
>  - remove the EP mode check function.
> 
>  drivers/pci/controller/dwc/Makefile|2 +-
>  drivers/pci/controller/dwc/pci-layerscape-ep.c |  147 
> 
>  2 files changed, 148 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
> 
> diff --git a/drivers/pci/controller/dwc/Makefile 
> b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..b26d617 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
>  obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c 
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> new file mode 100644
> index 000..289618b
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_DBI2_OFFSET 0x1000  /* DBI2 base address*/
> +
> +struct ls_pcie_ep {
> + struct dw_pcie  *pci;
> +};

I am not really sure why you need an additional struct.

> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)

Unused.

> +
> +static int ls_pcie_establish_link(struct dw_pcie *pci)
> +{
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops 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 void ls_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> + enum pci_barno bar;
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +
> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER;
> +}
> +
> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +   enum pci_epc_irq_type type, u16 interrupt_num)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_EPC_IRQ_LEGACY:
> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> + case PCI_EPC_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + case PCI_EPC_IRQ_MSIX:
> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + }
> +
> + return 0;

So if it falls through to the default, we log an error but return 0 ? This
does not make much sense.

I know you probably copy/pasted code from DWC platform, that code must
be fixed too I suppose.

Lorenzo

> +}
> +
> +static struct dw_pcie_ep_ops pcie_ep_ops = {
> + .ep_init = ls_pcie_ep_init,
> + .raise_irq = ls_pcie_ep_raise_irq,
> +};
> +
> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = pcie->pci;
> + struct device *dev = pci->dev;
> + struct dw_pcie_ep *ep;
> + struct resource *res;
> + int ret;
> +
> + ep = >ep;
> + ep->ops = _ep_ops;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space");
> + if (!res)
> + return -EINVAL;
> +
> + ep->phys_base = res->start;
> + ep->addr_size = resource_size(res);
> +
> + ret = dw_pcie_ep_init(ep);
> + if (ret) {
> + dev_err(dev, "failed to initialize endpoint\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int __init ls_pcie_ep_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct dw_pcie *pci;
> + struct ls_pcie_ep *pcie;
> + struct resource *dbi_base;
> + int ret;
> +
> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + dbi_base = platform_get_resource_byname(pdev, 

Re: [PATCHv2 3/6] PCI: layerscape: Add the EP mode support

[Lorenzo Pieralisi]

On Mon, Nov 05, 2018 at 04:46:50PM +0800, Xiaowei Bao wrote:
> Add the EP mode support.
> 
> Signed-off-by: Xiaowei Bao 
> ---
> v2:
>  - Add the SoC specific compatibles.
> 
>  .../devicetree/bindings/pci/layerscape-pci.txt |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Wrong commit $SUBJECT, this is not PCI code, it is a DT binding
update, I will have a look at the rest of the series to see if I
can update this patch or you will do it with the next respin.

Lorenzo

> diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt 
> b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> index 66df1e8..9c090c7 100644
> --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt
> @@ -13,12 +13,15 @@ information.
>  
>  Required properties:
>  - compatible: should contain the platform identifier such as:
> +  RC mode:
>  "fsl,ls1021a-pcie", "snps,dw-pcie"
>  "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie"
>  "fsl,ls2088a-pcie"
>  "fsl,ls1088a-pcie"
>  "fsl,ls1046a-pcie"
>  "fsl,ls1012a-pcie"
> +  EP mode:
> +"fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep"
>  - reg: base addresses and lengths of the PCIe controller register blocks.
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>entry for each entry in the interrupt-names property.
> -- 
> 1.7.1
> 

Re: [PATCHv4 0/3] dts: Add the property of IB and OB

[Lorenzo Pieralisi]

On Fri, Nov 10, 2017 at 11:48:44AM +0800, Bao Xiaowei wrote:
> Depend on http://patchwork.ozlabs.org/patch/815382/
> 
> Bao Xiaowei (3):
>   ARMv8: dts: ls1046a: add the property of IB and OB
>   ARMv8: layerscape: add the pcie ep function support
>   ARMv8: pcie: make the DWC EP driver support for layerscape
> 
>  arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi |   6 ++
>  drivers/pci/dwc/Kconfig|   1 +
>  drivers/pci/dwc/pci-layerscape.c   | 121 
> +++--
>  3 files changed, 121 insertions(+), 7 deletions(-)

Can I drop this series from the linux-pci patch queue ? I reckon
it is superseded by:

https://patchwork.ozlabs.org/patch/992928/

but I wanted to make sure that's the case, I really can't follow
the version numbering so I am asking.

Thanks,
Lorenzo

Re: [PATCHv4 1/3] ARMv8: dts: ls1046a: add the property of IB and OB

[Lorenzo Pieralisi]

On Mon, Nov 13, 2017 at 02:35:48AM +, M.h. Lian wrote:

[...]

> > > On Friday 10 November 2017 09:18 AM, Bao Xiaowei wrote:
> > > > Add the property of inbound and outbound windows number for ep driver.
> > > >
> > > > Signed-off-by: Bao Xiaowei 
> > > > Acked-by: Minghuan Lian 
> > > > ---
> > > >  v2:
> > > >  - no change
> > > >  v3:
> > > >  - modify the commit message
> > > >  v4:
> > > >  - no change
> > > >
> > > >  arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 6 ++
> > > >  1 file changed, 6 insertions(+)
> > >
> > > $subject should start with something like
> > > arm64: dts: ls1046a: **

Indeed.

> > > > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > > > b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > > > index 06b5e12d04d8..f8332669663c 100644
> > > > --- a/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > > > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi
> > > > @@ -674,6 +674,8 @@
> > > > device_type = "pci";
> > > > dma-coherent;
> > > > num-lanes = <4>;
> > > > +   num-ib-windows = <6>;
> > > > +   num-ob-windows = <6>;
> > >
> > > EP specific properties shouldn't be added in RC dt node. Ideally you
> > > should have a separate dt node for RC and EP.
> > 
> > It is a single PCIe controller which can be configured to either RC
> > mode or EP mode.  Wouldn't it conflict with the device tree
> > principles to have two device tree nodes for the same PCIe
> > controller?  And obviously the two modes cannot be used at the same
> > time so we cannot have two drivers both probe on the same hardware.
> > 
> [Minghuan Lian]  There is only one PCIe dts node in the dts file. PCIe
> dts node describes the PCIe controller's hardware properties and does
> not have work mode.  The new properties  "num-ib-windows " and
> "num-ob-windows" are used to describe the inbound/outbound window
> number included in the PCIe hardware. These windows are used in both
> RC and EP mode.  We can change work mode when resetting via RCW(reset
> configuration word).

I am not happy about this (that's what I am asking Rob to chime in
please on DT side).

1) I do not think it is allowed to have two DT nodes in a dts with same unit
   address (ie same reg property)

   
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/dra7.dtsi?h=v4.14

2) In the Synopsis Designware PCIe interface bindings we have some
   properties that are for RC mode and some for EP mode but there is
   no way from a *binding* perspective to detect in what mode the
   controller is:

   
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/designware-pcie.txt?h=v4.14

3) You can't use properties that in the bindings above are declared EP
   only for RC mode, we define bindings to respect their rules.

4) I think that a) a compatible should be added to the designware-pcie
   bindings to define endpoint mode and b) the same should be done for
   the ls1046a bindings. If the RC is programmed in EP mode DT firmware
   should be able to provide the information to an operating system, it
   is actually a _different_ component but on this I need DT people to
   chime in to define the best way forward.

I cannot review/merge this code until the points above are clarified.

Thanks,
Lorenzo

Re: [PATCH v2 1/1] Split VGA default device handler out of VGA arbiter

[Lorenzo Pieralisi]

On Thu, Aug 17, 2017 at 09:30:28PM +1000, Daniel Axtens wrote:
> A system without PCI legacy resources (e.g. ARM64) may find that no
> default/boot VGA device has been marked, because the VGA arbiter
> checks for legacy resource decoding before marking a card as default.

I do not understand this paragraph, in particular:

- "A system without PCI legacy resources (e.g. ARM64)". What does this
  mean ? I take this as "ARM64 does not support IO space"; if a PCI host
  bridge supports IO space, there is nothing from an architectural
  point of view that prevents an MMIO based IO space implementation to
  work on ARM64. It is PCI bridge specific, not arch specific.
- "the VGA arbiter checks for legacy resources". Do you mean it checks
  if a VGA class device enabled IO/MEM address spaces (and if the
  bridges upstream forwards those address spaces), correct ?

The assumption relying on ARM64 not supporting IO space (if that's what
you mean by "a system without PCI legacy resources") is not correct.

Thanks,
Lorenzo

> Split the small bit of code that does default VGA handling out from
> the arbiter. Add a Kconfig option to allow the kernel to be built
> with just the default handling, or the arbiter and default handling.
> 
> Add handling for devices that should be marked as default but aren't
> handled by the vga arbiter by adding a late initcall and a class
> enable hook. If there is no default from vgaarb then the first card
> that is enabled, has a driver bound, and can decode memory or I/O
> will be marked as default.
> 
> Signed-off-by: Daniel Axtens 
> 
> ---
> 
> v2: Tested on:
>  - x86_64 laptop
>  - arm64 D05 board with hibmc card
>  - qemu powerpc with tcg and bochs std-vga
> 
> I know this adds another config option and that's a bit sad, but
> we can't include it unconditionally as it depends on PCI.
> Suggestions welcome.
> ---
>  arch/ia64/pci/fixup.c|   2 +-
>  arch/powerpc/kernel/pci-common.c |   2 +-
>  arch/x86/pci/fixup.c |   2 +-
>  arch/x86/video/fbdev.c   |   2 +-
>  drivers/gpu/vga/Kconfig  |  12 +++
>  drivers/gpu/vga/Makefile |   1 +
>  drivers/gpu/vga/vga_default.c| 159 
> +++
>  drivers/gpu/vga/vga_switcheroo.c |   2 +-
>  drivers/gpu/vga/vgaarb.c |  41 +-
>  drivers/pci/pci-sysfs.c  |   2 +-
>  include/linux/vga_default.h  |  44 +++
>  include/linux/vgaarb.h   |  14 
>  12 files changed, 225 insertions(+), 58 deletions(-)
>  create mode 100644 drivers/gpu/vga/vga_default.c
>  create mode 100644 include/linux/vga_default.h
> 
> diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
> index 41caa99add51..b35d1cf4501a 100644
> --- a/arch/ia64/pci/fixup.c
> +++ b/arch/ia64/pci/fixup.c
> @@ -5,7 +5,7 @@
>  
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  #include 
> diff --git a/arch/powerpc/kernel/pci-common.c 
> b/arch/powerpc/kernel/pci-common.c
> index 341a7469cab8..4fd890a51d18 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -31,7 +31,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
>  #include 
>  #include 
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index 11e407489db0..b1254bc09a45 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -5,7 +5,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  
> diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
> index 9fd24846d094..62cfa74ea86e 100644
> --- a/arch/x86/video/fbdev.c
> +++ b/arch/x86/video/fbdev.c
> @@ -9,7 +9,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
>  int fb_is_primary_device(struct fb_info *info)
>  {
> diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
> index 29437eabe095..81d4105aecf6 100644
> --- a/drivers/gpu/vga/Kconfig
> +++ b/drivers/gpu/vga/Kconfig
> @@ -1,3 +1,14 @@
> +config VGA_DEFAULT
> + bool "VGA Default Device Support" if EXPERT
> + default y
> + depends on PCI
> + help
> +   Some programs find it helpful to know what VGA device is the default.
> +   On platforms like x86 this means the device used by the BIOS to show
> +   early boot messages. On other platforms this may be an arbitrary PCI
> +   graphics card. Select this to have a default device recorded within
> +   the kernel and exposed to userspace through sysfs.
> +
>  config VGA_ARB
>   bool "VGA Arbitration" if EXPERT
>   default y
> @@ -22,6 +33,7 @@ config VGA_SWITCHEROO
>   depends on X86
>   depends on ACPI
>   select VGA_ARB
> + select VGA_DEFAULT
>   help
> Many laptops released in 2008/9/10 have two GPUs with a multiplexer
> to switch between them. This adds support for dynamic switching when
> diff --git a/drivers/gpu/vga/Makefile b/drivers/gpu/vga/Makefile
> index 

Re: [RFC PATCH] Increase in idle power with schedutil

[Lorenzo Pieralisi]

On Sun, May 22, 2016 at 01:42:52PM -0700, Steve Muckle wrote:
> On Sun, May 22, 2016 at 12:39:12PM +0200, Peter Zijlstra wrote:
> > On Fri, May 20, 2016 at 05:53:41PM +0530, Shilpasri G Bhat wrote:
> > > 
> > > Below are the comparisons by disabling watchdog.
> > > Both schedutil and ondemand have a similar ramp-down trend. And in both 
> > > the
> > > cases I can see that frequency of the cpu is not reduced in deterministic
> > > fashion. In a observation window of 30 seconds after running a workload I 
> > > can
> > > see that the frequency is not ramped down on some cpus in the system and 
> > > are
> > > idling at max frequency.
> > 
> > So does it actually matter what the frequency is when you idle? Isn't
> > the whole thing clock gated anyway?
> > 
> > Because this seems to generate contradictory requirements, on the one
> > hand we want to stay idle as long as possible while on the other hand
> > you seem to want to clock down while idle, which requires not being
> > idle.
> > 
> > If it matters; should not your idle state muck explicitly set/restore
> > frequency?
> 
> AFAIK this is very platform dependent. Some will waste more power than
> others when a CPU idles above fmin due to things like resource (bus
> bandwidth, shared cache freq etc) voting.

It is also related to static leakage power that depends on the operating
voltage (ie higher operating frequencies require higher voltage) so in a
way scaling frequency before going idle may not be effective if voltage
does not scale too in turn.

Lorenzo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v9 5/6] PCI: generic: Make pci-host-generic driver numa aware

[Lorenzo Pieralisi]

On Tue, Jan 19, 2016 at 11:28:56AM +0530, Ganapatrao Kulkarni wrote:
> On Mon, Jan 18, 2016 at 11:11 PM, David Daney  wrote:
> > On 01/18/2016 08:36 AM, Ganapatrao Kulkarni wrote:
> >>
> >> update numa_node of device associated with pci bus.
> >> moved down devm_kzalloc to allocate from node memory.
> >>
> >> Signed-off-by: Ganapatrao Kulkarni 
> >> ---
> >>   drivers/pci/host/pci-host-generic.c | 9 ++---
> >>   1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/pci/host/pci-host-generic.c
> >> b/drivers/pci/host/pci-host-generic.c
> >> index 5434c90..0e1ce06 100644
> >> --- a/drivers/pci/host/pci-host-generic.c
> >> +++ b/drivers/pci/host/pci-host-generic.c
> >> @@ -215,11 +215,9 @@ static int gen_pci_probe(struct platform_device
> >> *pdev)
> >> const struct of_device_id *of_id;
> >> struct device *dev = >dev;
> >> struct device_node *np = dev->of_node;
> >> -   struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> >> +   struct gen_pci *pci;
> >> struct pci_bus *bus, *child;
> >>
> >> -   if (!pci)
> >> -   return -ENOMEM;
> >>
> >> type = of_get_property(np, "device_type", NULL);
> >> if (!type || strcmp(type, "pci")) {
> >> @@ -230,6 +228,11 @@ static int gen_pci_probe(struct platform_device
> >> *pdev)
> >> of_pci_check_probe_only();
> >>
> >> of_id = of_match_node(gen_pci_of_match, np);
> >> +   set_dev_node(dev, of_node_to_nid(np));
> >
> >
> > This shouldn't be done in individual platform_drivers, but instead in the
> > device probing code.
> >
> > There is code that does this in drivers/of/platform.c and
> > drivers/of/device.c  Is that not being called for the pci-host-gweneric
> > devices?  If not, we should figure out why, and perhaps attempt to fix it
> > rather than doing it here.
> is it more appropriate to call of_platform_device_create ?

That's already done to create the platform device by OF core when
populating devices from DT, what David suggested is that you should
add set_dev_node() to core OF code instead of adding it specifically
to the PCI host generic code.

Lorenzo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 00/16] Refine PCI host bridge scan interfaces

[Lorenzo Pieralisi]

On Tue, Nov 18, 2014 at 11:30:11AM +, Arnd Bergmann wrote:
 On Tuesday 18 November 2014 19:17:32 Yijing Wang wrote:
  On 2014/11/17 22:13, Arnd Bergmann wrote:
   On Monday 17 November 2014 18:21:34 Yijing Wang wrote:
   This series is based Linux 3.18-rc1 and Lorenzo Pieralisi's
   arm PCI domain cleanup patches, link: 
   https://patchwork.ozlabs.org/patch/407585/
  
   Current pci scan interfaces like pci_scan_root_bus() and directly
   call pci_create_root_bus()/pci_scan_child_bus() lack flexiblity.
   Some platform infos like PCI domain and msi_chip have to be
   associated to PCI bus by some arch specific function.
   We want to make a generic pci_host_bridge, and make it hold
   the platform infos or hook. Then we could eliminate the lots
   of arch pci_domain_nr, also we could associate some platform 
   ops something like pci_get_msi_chip(struct pci_dev *dev)
   with pci_host_bridge to avoid introduce arch weak functions.
  
   This RFC version not for all platforms, just applied the new
   scan interface in x86/arm/powerpc/ia64, I will refresh other
   platforms after the core pci scan interfaces are ok.
   
   I think overall this is a good direction to take, in particular
   moving more things into struct pci_host_bridge so we can
   slim down the architecture specific code.
  
  Hi Arnd, thanks very much for your review and comments!
  
   
   I don't particularly like the way you use the 'pci_host_info'
   to pass callback pointers and some of the generic information.
   This duplicates some of the issues we are currently trying
   to untangle in the arm32 code to make drivers easier to share
   between architectures.
  
  What arm32 code you are trying to untangle for example ?
 
 We have a few problems that currently prevent us from using shared
 drivers across arm32 and arm64:
 
 - arm32 has an architecture-defined pci_sys_data structure, but
   we really want to have one that is defined by the host bridge driver
   and that is architecture independent. Some core functions depend
   on this structure at the moment, which Lorenzo is trying to
   undo

Yes, and on this specific point I would like to understand why we
are adding yet more pci_sys_data data in the last series that is
already in -next:

https://lkml.org/lkml/2014/10/27/85

What does this buy us ? The cover letter says already that there *is*
a better solution, why do not we work on that instead of adding more churn
to arch specific code ?

Thanks,
Lorenzo
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 14/16] arm/PCI: Introduce pci_get_domain_nr()

[Lorenzo Pieralisi]

On Mon, Nov 17, 2014 at 10:21:48AM +, Yijing Wang wrote:
 Signed-off-by: Yijing Wang wangyij...@huawei.com
 ---
  arch/arm/include/asm/mach/pci.h |9 +
  arch/arm/kernel/bios32.c|8 ++--
  2 files changed, 15 insertions(+), 2 deletions(-)
 
 diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
 index f19f627..370b3bd 100644
 --- a/arch/arm/include/asm/mach/pci.h
 +++ b/arch/arm/include/asm/mach/pci.h
 @@ -90,6 +90,15 @@ extern void pci_map_io_early(unsigned long pfn);
  static inline void pci_map_io_early(unsigned long pfn) {}
  #endif
  
 +#ifdef CONFIG_PCI_DOMAINS_GENERIC
 +int pci_get_domain_nr(struct device *parent)
 +#else
 +static inline int pci_get_domain_nr(struct device *parent)
 +{
 + return 0;
 +}
 +#endif
 +
  /*
   * PCI controllers
   */
 diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
 index d8c2b4e..3fe56f1 100644
 --- a/arch/arm/kernel/bios32.c
 +++ b/arch/arm/kernel/bios32.c
 @@ -513,7 +513,7 @@ static void pcibios_init_hw(struct device *parent, struct 
 hw_pci *hw,
  #ifdef CONFIG_PCI_DOMAINS_GENERIC
  static bool dt_domain_found;
  
 -void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
 +int pci_get_domain_nr(struct device *parent)
  {
   int domain = of_get_pci_domain_nr(parent-of_node);
  
 @@ -526,8 +526,12 @@ void pci_bus_assign_domain_nr(struct pci_bus *bus, 
 struct device *parent)
   } else {
   domain = pci_get_new_domain_nr();
   }
 + return domain;
 +}
  
 - bus-domain_nr = domain;
 +void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent)
 +{
 + bus-domain_nr = pci_get_domain_nr(parent);
  }
  #endif

This code is superseded by the last patches I sent to move the domain
assignment to PCI core code.

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/301220.html

Lorenzo

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC PATCH 09/16] PCI: Associate .get_msi_ctrl() with pci_host_bridge

[Lorenzo Pieralisi]

On Mon, Nov 17, 2014 at 10:21:43AM +, Yijing Wang wrote:
 From: Yijing Wang wangyijing0...@gmail.com
 
 Signed-off-by: Yijing Wang wangyij...@huawei.com
 ---
  drivers/pci/host-bridge.c |1 +
  include/linux/pci.h   |2 ++
  2 files changed, 3 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
 index 49b6c21..872cae1 100644
 --- a/drivers/pci/host-bridge.c
 +++ b/drivers/pci/host-bridge.c
 @@ -58,6 +58,7 @@ struct pci_host_bridge *pci_create_host_bridge(
   host-dev.parent = parent;
   INIT_LIST_HEAD(host-windows);
   host-dev.release = pci_release_host_bridge_dev;
 + host-get_msi_ctrl = info-get_msi_ctrl;
  
   /* this is hack, just for build, will be removed later*/
   b = kzalloc(sizeof(*b), GFP_KERNEL);
 diff --git a/include/linux/pci.h b/include/linux/pci.h
 index a51f5f5..af1ee86 100644
 --- a/include/linux/pci.h
 +++ b/include/linux/pci.h
 @@ -408,6 +408,7 @@ struct pci_host_bridge {
   int domain;
   void *sysdata;
   struct pci_ops *ops;
 + struct msi_controller *(*get_msi_ctrl)(struct pci_dev *pdev);
   void (*release_fn)(struct pci_host_bridge *);
   void *release_data;
  };
 @@ -416,6 +417,7 @@ struct pci_host_info {
   u8 res_type;
   void *arg;
   struct list_head *resources; /*just for build, will clean up later */
 + struct msi_controller *(*get_msi_ctrl)(struct pci_dev *pdev);
   int (*init_res)(struct pci_host_bridge *host, 
   struct pci_host_info *info);
  };

Where would you use the get_msi_ctrl pointer then ? Wasn't it better
to wait for this patchset to take shape before adding more churn to
the ARM (and other archs) pci_sys_data structure and consequently add
another pcibios call to achieve what the get_msi_ctrl pointer is there to
achieve (ie making msi_controller retrieval arch independent ?)

I just do not see what the pci_sys_data intermediate step buys you if
we consider the approach taken in this patch as the proper solution.

Thanks,
Lorenzo

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] cpuidle/powernv: Populate cpuidle state details by querying the device-tree

[Lorenzo Pieralisi]

On Tue, Oct 14, 2014 at 08:53:00AM +0100, Preeti U Murthy wrote:
 We hard code the metrics relevant for cpuidle states in the kernel today.
 Instead pick them up from the device tree so that they remain relevant
 and updated for the system that the kernel is running on.
 
 Cc: linux...@vger.kernel.org
 Cc: Rafael J. Wysocki r...@rjwysocki.net
 Cc: devicet...@vger.kernel.org
 Cc: linuxppc-dev@lists.ozlabs.org
 Cc: Michael Ellerman m...@ellerman.id.au
 Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
 Signed-off-by: Preeti U. Murthy pre...@linux.vnet.ibm.com
 Signed-off-by: Shreyas B. Prabhu shre...@linux.vnet.ibm.com
 ---
 
  drivers/cpuidle/cpuidle-powernv.c |   27 ++-
  1 file changed, 22 insertions(+), 5 deletions(-)

Device tree properties should be documented, and these bindings are
getting very similar to the ones I have just completed for ARM,
I wonder whether we should take the generic bits out of ARM bindings (ie
exit_latency) and make those available to other architectures.

Thanks,
Lorenzo

 
 diff --git a/drivers/cpuidle/cpuidle-powernv.c 
 b/drivers/cpuidle/cpuidle-powernv.c
 index fa79392..b57681d 100644
 --- a/drivers/cpuidle/cpuidle-powernv.c
 +++ b/drivers/cpuidle/cpuidle-powernv.c
 @@ -165,7 +165,8 @@ static int powernv_add_idle_states(void)
   int nr_idle_states = 1; /* Snooze */
   int dt_idle_states;
   const __be32 *idle_state_flags;
 - u32 len_flags, flags;
 + const __be32 *idle_state_latency;
 + u32 len_flags, flags, latency_ns;
   int i;
  
   /* Currently we have snooze statically defined */
 @@ -182,18 +183,32 @@ static int powernv_add_idle_states(void)
   return nr_idle_states;
   }
  
 + idle_state_latency = of_get_property(power_mgt,
 + ibm,cpu-idle-state-latencies-ns, NULL);
 + if (!idle_state_latency) {
 + pr_warn(DT-PowerMgmt: missing 
 ibm,cpu-idle-state-latencies-ns\n);
 + return nr_idle_states;
 + }
 +
   dt_idle_states = len_flags / sizeof(u32);
  
   for (i = 0; i  dt_idle_states; i++) {
  
   flags = be32_to_cpu(idle_state_flags[i]);
 +
 + /* Cpuidle accepts exit_latency in us and we estimate
 +  * target residency to be 10x exit_latency
 +  */
 + latency_ns = be32_to_cpu(idle_state_latency[i]);
   if (flags  IDLE_USE_INST_NAP) {
   /* Add NAP state */
   strcpy(powernv_states[nr_idle_states].name, Nap);
   strcpy(powernv_states[nr_idle_states].desc, Nap);
   powernv_states[nr_idle_states].flags = 
 CPUIDLE_FLAG_TIME_VALID;
 - powernv_states[nr_idle_states].exit_latency = 10;
 - powernv_states[nr_idle_states].target_residency = 100;
 + powernv_states[nr_idle_states].exit_latency =
 + ((unsigned int)latency_ns) / 1000;
 + powernv_states[nr_idle_states].target_residency =
 + ((unsigned int)latency_ns / 100);
   powernv_states[nr_idle_states].enter = nap_loop;
   nr_idle_states++;
   }
 @@ -204,8 +219,10 @@ static int powernv_add_idle_states(void)
   strcpy(powernv_states[nr_idle_states].desc, 
 FastSleep);
   powernv_states[nr_idle_states].flags =
   CPUIDLE_FLAG_TIME_VALID | 
 CPUIDLE_FLAG_TIMER_STOP;
 - powernv_states[nr_idle_states].exit_latency = 300;
 - powernv_states[nr_idle_states].target_residency = 
 100;
 + powernv_states[nr_idle_states].exit_latency =
 + ((unsigned int)latency_ns) / 1000;
 + powernv_states[nr_idle_states].target_residency =
 + ((unsigned int)latency_ns / 100);
   powernv_states[nr_idle_states].enter = fastsleep_loop;
   nr_idle_states++;
   }
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-pm in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v3 04/27] arm/MSI: Save MSI chip in pci_sys_data

[Lorenzo Pieralisi]

On Wed, Oct 15, 2014 at 04:06:52AM +0100, Yijing Wang wrote:
 Saving msi chip in pci_sys_data can make pci bus and
 devices don't need to know msi chip detail, it also
 make pci enumeration code be decoupled from msi chip.
 In fact, all pci devices under the same pci hostbridge
 share same msi chip. So msi chip should be seen as one
 of resources or attributes to be initialized in pci host
 bridge driver. Currently, pci hostbridge drivers create
 pci_host_bridge in pci_create_root_bus(), and pass arch
 specific pci sysdata to core pci scan functions. So pci
 arch sysdata is good place to save msi chip.
 
 Signed-off-by: Yijing Wang wangyij...@huawei.com
 ---
  arch/arm/include/asm/mach/pci.h |6 ++
  arch/arm/include/asm/pci.h  |9 +
  arch/arm/kernel/bios32.c|3 +++
  drivers/pci/msi.c   |6 ++
  include/linux/pci.h |9 +
  5 files changed, 33 insertions(+), 0 deletions(-)
 
 diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
 index 7fc4278..59b0d87 100644
 --- a/arch/arm/include/asm/mach/pci.h
 +++ b/arch/arm/include/asm/mach/pci.h
 @@ -22,6 +22,9 @@ struct hw_pci {
  #ifdef CONFIG_PCI_DOMAINS
   int domain;
  #endif
 +#ifdef CONFIG_PCI_MSI
 + struct msi_chip *msi_chip;
 +#endif
   struct pci_ops  *ops;
   int nr_controllers;
   void**private_data;
 @@ -47,6 +50,9 @@ struct pci_sys_data {
  #ifdef CONFIG_PCI_DOMAINS
   int domain;
  #endif
 +#ifdef CONFIG_PCI_MSI
 + struct msi_chip *msi_chip;
 +#endif

This struct is defined for ARM only. We are trying to remove dependency on it
so that we can have a single driver for ARM32 and ARM64 systems without
pcibios dependency. Why can't we add this msi chip data to the host
bridge struct instead ? I understand it is all a matter of how to pass
the pointer to pci_scan_root_bus(), where the bridge is created.
We could initialize the msi chip on the bridge through the bus pointer
returned by the pci_scan_root_bus() function.

Thanks,
Lorenzo

   struct list_head node;
   int busnr;  /* primary bus number   
 */
   u64 mem_offset; /* bus-cpu memory mapping offset   
 */
 diff --git a/arch/arm/include/asm/pci.h b/arch/arm/include/asm/pci.h
 index 7e95d85..b562c09 100644
 --- a/arch/arm/include/asm/pci.h
 +++ b/arch/arm/include/asm/pci.h
 @@ -31,6 +31,15 @@ static inline int pci_proc_domain(struct pci_bus *bus)
  }
  #endif /* CONFIG_PCI_DOMAINS */
  
 +#ifdef CONFIG_PCI_MSI
 +static inline struct msi_chip *pci_msi_chip(struct pci_bus *bus)
 +{
 + struct pci_sys_data *root = bus-sysdata;
 +
 + return root-msi_chip;
 +}
 +#endif
 +
  /*
   * The PCI address space does equal the physical memory address space.
   * The networking and block device layers use this boolean for bounce
 diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
 index 17a26c1..a19038d 100644
 --- a/arch/arm/kernel/bios32.c
 +++ b/arch/arm/kernel/bios32.c
 @@ -471,6 +471,9 @@ static void pcibios_init_hw(struct device *parent, struct 
 hw_pci *hw,
  #ifdef CONFIG_PCI_DOMAINS
   sys-domain  = hw-domain;
  #endif
 +#ifdef CONFIG_PCI_MSI
 + sys-msi_chip = hw-msi_chip;
 +#endif
   sys-busnr   = busnr;
   sys-swizzle = hw-swizzle;
   sys-map_irq = hw-map_irq;
 diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
 index 22e413c..f11108c 100644
 --- a/drivers/pci/msi.c
 +++ b/drivers/pci/msi.c
 @@ -35,6 +35,9 @@ int __weak arch_setup_msi_irq(struct pci_dev *dev, struct 
 msi_desc *desc)
   struct msi_chip *chip = dev-bus-msi;
   int err;
  
 + if (!chip)
 + chip = pci_msi_chip(dev-bus);
 +
   if (!chip || !chip-setup_irq)
   return -EINVAL;
  
 @@ -50,6 +53,9 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
   struct msi_desc *entry = irq_get_msi_desc(irq);
   struct msi_chip *chip = entry-dev-bus-msi;
  
 + if (!chip)
 + chip = pci_msi_chip(entry-dev-bus);
 +
   if (!chip || !chip-teardown_irq)
   return;
  
 diff --git a/include/linux/pci.h b/include/linux/pci.h
 index 9cd2721..7a48b40 100644
 --- a/include/linux/pci.h
 +++ b/include/linux/pci.h
 @@ -1433,6 +1433,15 @@ static inline int pci_get_new_domain_nr(void) { return 
 -ENOSYS; }
  
  #include asm/pci.h
  
 +/* Just avoid compile error, will be clean up later */
 +#ifdef CONFIG_PCI_MSI
 +
 +#ifndef pci_msi_chip
 +#define pci_msi_chip(bus)NULL
 +#endif
 +
 +#endif
 +
  /* these helpers provide future and backwards compatibility
   * for accessing popular PCI BAR info */
  #define pci_resource_start(dev, bar) ((dev)-resource[(bar)].start)
 -- 
 1.7.1
 
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  

Re: [RFC PATCH v2 3/4] powerpc: refactor of_get_cpu_node to support other architectures

[Lorenzo Pieralisi]

On Wed, Aug 28, 2013 at 08:46:38PM +0100, Grant Likely wrote:
 On Thu, 22 Aug 2013 14:59:30 +0100, Mark Rutland mark.rutl...@arm.com wrote:
  On Mon, Aug 19, 2013 at 02:56:10PM +0100, Sudeep KarkadaNagesha wrote:
   On 19/08/13 14:02, Rob Herring wrote:
On 08/19/2013 05:19 AM, Mark Rutland wrote:
On Sat, Aug 17, 2013 at 11:09:36PM +0100, Benjamin Herrenschmidt wrote:
On Sat, 2013-08-17 at 12:50 +0200, Tomasz Figa wrote:
I wonder how would this handle uniprocessor ARM (pre-v7) cores, for
which 
the updated bindings[1] define #address-cells = 0 and so no reg 
property.
   
[1] - http://thread.gmane.org/gmane.linux.ports.arm.kernel/260795
   
Why did you do that in the binding ? That sounds like looking to 
create
problems ... 
   
Traditionally, UP setups just used 0 as the reg property on other
architectures, why do differently ?
   
The decision was taken because we defined our reg property to refer to
the MPIDR register's Aff{2,1,0} bitfields, and on UP cores before v7
there's no MPIDR register at all. Given there can only be a single CPU
in that case, describing a register that wasn't present didn't seem
necessary or helpful.

What exactly reg represents is up to the binding definition, but it
still should be present IMO. I don't see any issue with it being
different for pre-v7.

   Yes it's better to have 'reg' with value 0 than not having it.
   Otherwise this generic of_get_cpu_node implementation would need some
   _hack_ to handle that case.
  
  I'm not sure that having some code to handle a difference in standard
  between two architectures is a hack. If anything, I'd argue encoding a
  reg of 0 that corresponds to a nonexistent MPIDR value (given that's
  what the reg property is defined to map to on ARM) is more of a hack ;)
  
  I'm not averse to having a reg value of 0 for this case, but given that
  there are existing devicetrees without it, requiring a reg property will
  break compatibility with them.
 
 Then special cases those device trees, but you changing existing
 convention really needs to be avoided. The referenced documentation
 change is brand new, so we're not stuck with it.

I have no problem with changing the bindings and forcing:

#address-cells = 1;
reg = 0;

for UP predating v7, my big worry is related to in-kernel dts that we
already patched to follow the #address-cells = 0 rule (and we had to
do it since we got asked that question multiple times on the public
lists).

What do you mean by special case those device trees ? I have not
planned to patch them again, unless we really consider that a necessary
evil.

Thanks,
Lorenzo

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev