From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: kishon@ti.com, lorenzo.pieralisi@arm.com, bhelgaas@google.com,
robh@kernel.org, devicetree@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, hemantk@codeaurora.org,
smohanad@codeaurora.org, bjorn.andersson@linaro.org,
sallenki@codeaurora.org, skananth@codeaurora.org,
vpernami@codeaurora.org, vbadigan@codeaurora.org
Subject: Re: [PATCH v6 2/3] PCI: dwc: Add Qualcomm PCIe Endpoint controller driver
Date: Thu, 22 Jul 2021 17:24:04 +0530 [thread overview]
Message-ID: <20210722115404.GB4446@workstation> (raw)
In-Reply-To: <20210714191509.GA1864706@bjorn-Precision-5520>
On Wed, Jul 14, 2021 at 02:15:09PM -0500, Bjorn Helgaas wrote:
> Can you use the driver name for *this* driver in the subject instead
> of "dwc"? Then we'll be able to identify changes related to this
> driver easily in the "git log --oneline" output.
>
Okay sure. I usually do that for the drivers that got merged but don't
have any issues with driver prefix while under review.
> I'm not sure what that name should be -- you have the PCIE_QCOM_EP
> config symbol and "qcom-pcie-ep" as the driver.name. Both seem
> possibly a little too generic. We already have a "qcom" driver
> (drivers/pci/controller/dwc/pcie-qcom.c). Is this for the same
> hardware in endpoint mode?
>
Yes
> Will this driver support every endpoint device from Qualcomm, even
> future ones? People often use a model or codename here (xgene,
> aardvark, armada, tegra, etc). But I guess you can always use
> something more specific for future drivers if/when Qualcomm produces
> something that requires a different driver.
>
Since pcie-qcom.c supports most of the RC IPs in recent SoCs, I can
assure that this driver can handle all EPs.
> On Wed, Jul 14, 2021 at 02:03:15PM +0530, Manivannan Sadhasivam wrote:
> > Add driver support for Qualcomm PCIe Endpoint controller driver based on
> > the Designware core with added Qualcomm specific wrapper around the
> > core. The driver support is very basic such that it supports only
> > enumeration, PCIe read/write, and MSI. There is no ASPM and PM support
> > for now but these will be added later.
> >
> > The driver is capable of using the PERST# and WAKE# side-band GPIOs for
> > operation and written on top of the DWC PCI framework.
> >
[...]
> > +static int qcom_pcie_establish_link(struct dw_pcie *pci)
> > +{
>
> This is much more complicated than existing *_pcie_establish_link()
> functions. Are other drivers doing work like this in different
> functions?
>
> Please try to copy the same structure and function name conventions as
> other drivers. This applies to all the functions here. It makes
> maintenance much easier if code doing similar things looks similar.
>
Sure thing. I can use the patterns for the callback functions as they
are the ones shared between all drivers.
Thanks,
Mani
next prev parent reply other threads:[~2021-07-22 11:54 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-14 8:33 [PATCH v6 0/3] Add Qualcomm PCIe Endpoint driver support Manivannan Sadhasivam
2021-07-14 8:33 ` [PATCH v6 1/3] dt-bindings: pci: Add devicetree binding for Qualcomm PCIe EP controller Manivannan Sadhasivam
2021-07-14 8:33 ` [PATCH v6 2/3] PCI: dwc: Add Qualcomm PCIe Endpoint controller driver Manivannan Sadhasivam
2021-07-14 19:15 ` Bjorn Helgaas
2021-07-22 11:54 ` Manivannan Sadhasivam [this message]
2021-07-14 8:33 ` [PATCH v6 3/3] MAINTAINERS: Add entry for Qualcomm PCIe Endpoint driver and binding Manivannan Sadhasivam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210722115404.GB4446@workstation \
--to=manivannan.sadhasivam@linaro.org \
--cc=bhelgaas@google.com \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=helgaas@kernel.org \
--cc=hemantk@codeaurora.org \
--cc=kishon@ti.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=robh@kernel.org \
--cc=sallenki@codeaurora.org \
--cc=skananth@codeaurora.org \
--cc=smohanad@codeaurora.org \
--cc=vbadigan@codeaurora.org \
--cc=vpernami@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.