Hi Bjorn,
Thanks for the review.
> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: Friday, January 22, 2021 1:22 AM
> To: Thokala, Srikanth <srikanth.thokala@xxxxxxxxx>
> Cc: bhelgaas@xxxxxxxxxx; robh+dt@xxxxxxxxxx; lorenzo.pieralisi@xxxxxxx;
> linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> andriy.shevchenko@xxxxxxxxxxxxxxx; mgross@xxxxxxxxxxxxxxx; Raja
> Subramanian, Lakshmi Bai <lakshmi.bai.raja.subramanian@xxxxxxxxx>;
> Sangannavar, Mallikarjunappa <mallikarjunappa.sangannavar@xxxxxxxxx>
> Subject: Re: [PATCH v6 2/2] PCI: keembay: Add support for Intel Keem Bay
>
> On Fri, Jan 22, 2021 at 08:56:10AM +0530, srikanth.thokala@xxxxxxxxx
> wrote:
> > From: Srikanth Thokala <srikanth.thokala@xxxxxxxxx>
> >
> > Add driver for Intel Keem Bay SoC PCIe controller. This controller
> > is based on DesignWare PCIe core.
> >
> > In root complex mode, only internal reference clock is possible for
> > Keem Bay A0. For Keem Bay B0, external reference clock can be used
> > and will be the default configuration. Currently, keembay_pcie_of_data
> > structure has one member. It will be expanded later to handle this
> > difference.
> >
> > Endpoint mode link initialization is handled by the boot firmware.
> >
> > Signed-off-by: Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@xxxxxxxxx>
> > Signed-off-by: Srikanth Thokala <srikanth.thokala@xxxxxxxxx>
> > Acked-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > MAINTAINERS | 7 +
> > drivers/pci/controller/dwc/Kconfig | 28 ++
> > drivers/pci/controller/dwc/Makefile | 1 +
> > drivers/pci/controller/dwc/pcie-keembay.c | 446 ++++++++++++++++++++++
> > 4 files changed, 482 insertions(+)
> > create mode 100644 drivers/pci/controller/dwc/pcie-keembay.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 00836f6452f0..2fc0fb03c430 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13852,6 +13852,13 @@ L: linux-pci@xxxxxxxxxxxxxxx
> > S: Maintained
> > F: drivers/pci/controller/dwc/*spear*
> >
> > +PCI DRIVER FOR INTEL KEEM BAY
> > +M: Srikanth Thokala <srikanth.thokala@xxxxxxxxx>
> > +L: linux-pci@xxxxxxxxxxxxxxx
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/pci/intel,keembay-pcie*
> > +F: drivers/pci/controller/dwc/pcie-keembay.c
>
> <checks MAINTAINERS> ... yep, all previous entries are in alphabetical
> order. This new one just got dropped at the end.
>
> I feel like a broken record, but please, please, take a look at the
> surrounding code/text/whatever, and MAKE YOUR NEW STUFF MATCH THE
> EXISTING STYLE. We want the whole thing to be reasonably consistent
> so readers can make sense of it without being confused by the
> idiosyncrasies of every contributor.
>
> Also, probably s/PCI DRIVER/PCIE DRIVER/. We have both (an existing
> inconsistency), but pick one, put it in the section that matches, and
> alphabetize.
Sorry I missed; I will fix it in my next version.
>
> > +
> > PCMCIA SUBSYSTEM
> > M: Dominik Brodowski <linux@xxxxxxxxxxxxxxxxxxxx>
> > S: Odd Fixes
>
> > +static void keembay_ep_reset_deassert(struct keembay_pcie *pcie)
> > +{
> > + msleep(100);
>
> Please note the spec section that requires this sleep. Otherwise it's
> just an unmaintainable magic number.
Sure, I will do that.
Thanks!
Srikanth
>
> > + gpiod_set_value_cansleep(pcie->reset, 0);
> > + usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
> > +}
