Hi Bjorn,

Thanks a lot for your comments!

> -----Original Message-----
> From: Bjorn Helgaas [mailto:[email protected]]
> Sent: 2019年4月12日 22:01
> To: Z.q. Hou <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Leo Li <[email protected]>;
> [email protected]; [email protected];
> [email protected]; Mingkai Hu <[email protected]>; M.h. Lian
> <[email protected]>; Xiaowei Bao <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > Reviewed-by: Minghuan Lian <[email protected]>
> > ---
> > 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

Reply via email to