RE: [PATCH v6 2/2] PCI: keembay: Add support for Intel Keem Bay

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);
> > +}




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux