RE: [EXT] Re: [PATCHv5 4/6] PCI: mobiveil: Add PCIe Gen4 RC driver for NXP Layerscape SoCs

[Z.q. Hou]

Hi Bjorn,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
> Sent: 2019å4æ12æ 22:01
> To: Z.q. Hou <zhiqiang.hou@xxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; l.subrahmanya@xxxxxxxxxxxxxx;
> shawnguo@xxxxxxxxxx; Leo Li <leoyang.li@xxxxxxx>;
> lorenzo.pieralisi@xxxxxxx; catalin.marinas@xxxxxxx;
> will.deacon@xxxxxxx; Mingkai Hu <mingkai.hu@xxxxxxx>; M.h. Lian
> <minghuan.lian@xxxxxxx>; Xiaowei Bao <xiaowei.bao@xxxxxxx>
> Subject: [EXT] Re: [PATCHv5 4/6] PCI: mobiveil: Add PCIe Gen4 RC driver for
> NXP Layerscape SoCs
> 
> WARNING: This email was created outside of NXP. DO NOT CLICK links or
> attachments unless you recognize the sender and know the content is safe.
> 
> 
> 
> On Fri, Apr 12, 2019 at 09:52:50AM +0000, Z.q. Hou wrote:
> > From: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx>
> >
> > This PCIe controller is based on the Mobiveil GPEX IP, which is
> > compatible with the PCI Expressâ Base Specification, Revision 4.0.
> >
> > Signed-off-by: Hou Zhiqiang <Zhiqiang.Hou@xxxxxxx>
> > Reviewed-by: Minghuan Lian <Minghuan.Lian@xxxxxxx>
> > ---
> > V5:
> >  - Corrected the subject.
> >  - Corrected 2 typos.
> >  - Updated the Copyright and driver description.
> >  - Changed to use BIT(x) to define verious functions of register's bits.
> >  - Unified the capitalization of error info.
> >  - Changed the IRQ handler name to ls_pcie_g4_isr().
> >  - Change 'irq' to 'IRQ' in error info.
> >  - Trimmed some functions without functionality change.
> >
> >  drivers/pci/controller/mobiveil/Kconfig       |  10 +
> >  drivers/pci/controller/mobiveil/Makefile      |   1 +
> 
> >  .../controller/mobiveil/pci-layerscape-gen4.c | 256
> > ++++++++++++++++++
> 
> I would probably name this "pcie-layerscape-gen4.c" ("pcie" instead of "pci"),
> since that's more typical and this really is PCIe-specific.

Yes, this can be arranged in v6.

> 
> > +#define PCIE_PF_DBG                  0x7fc
> > +#define PF_DBG_LTSSM_MASK            0x3f
> > +#define PF_DBG_WE                    BIT(31)
> > +#define PF_DBG_PABR                  BIT(27)
> > +
> > +#define LS_PCIE_G4_LTSSM_L0          0x2d /* L0 state */
> 
> Maybe rename this and move it to make it obvious that it's related to
> PF_DBG_LTSSM_MASK?

Yes, will rename it in v6.

> 
> > +             dev_err(dev, "Poll PABRST&PABACT timeout.\n");
> 
> No need for punctuation at end of messages.

Will remove them in v6.

Thanks,
Zhiqiang