linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jim Quinlan <james.quinlan@broadcom.com>
To: Mark Brown <broonie@kernel.org>
Cc: "open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" 
	<linux-pci@vger.kernel.org>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>,
	"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
	<bcm-kernel-feedback-list@broadcom.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Rob Herring <robh@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-rpi-kernel@lists.infradead.org>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v1 2/6] PCI: brcmstb: Add control of EP voltage regulator(s)
Date: Fri, 27 Nov 2020 15:26:53 -0500	[thread overview]
Message-ID: <CA+-6iNzJAf_bKVjbw8bkh3qmSU++m6-DoFKQvBTTZGonYJGXfg@mail.gmail.com> (raw)
In-Reply-To: <20201126114912.GA8506@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 1980 bytes --]

On Thu, Nov 26, 2020 at 6:49 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Nov 25, 2020 at 02:24:19PM -0500, Jim Quinlan wrote:
>
> > +     for (i = 0; i < PCIE_REGULATORS_MAX; i++) {
> > +             ep_reg = devm_regulator_get_optional(dev, ep_regulator_names[i]);
> > +             if (IS_ERR(ep_reg)) {
>
> Does PCI allow supplies to be physically absent?  If not then the driver
> shouldn't be using regulator_get_optional() and much of the code here
> can be deleted.
Hi Mark,

First, as an aside, I'm  a little confused about the purpose of
devm_regulator_get_optional(...);  the other  xxx_get_optional() calls
I am familiar with (eg clock, reset, gpio) return NULL if the desired
item does not exist, and then NULL can be used as a valid pointer for
the rest of the API.  Not so here.

At any rate, our SOCs are placed in a variety of boards which
implement the PCIe RC-EP connection as they wish.  From the PCIe
driver's point of view, the type of power supply that needs to be
turned on is specified in the DT and they cannot be hard coded by the
driver.  I've listed all of the four possibilities; typically one,
maybe two will  be specified, but never all of them.  In addition,
sometimes a regulator is hard wired on and not even controllable by
the PCIe controller.

>
> > +static void brcm_set_regulators(struct brcm_pcie *pcie, bool on)
> > +{
>
> This is open coding the regulator bulk APIs.
Except that a bulk regulator "get"  requires that all supplies are
present.  I would have to first scan the node's properties for the
"-supply" properties and fill in the bulk regulator structure.  I'm
fine with doing that.

However, a previous incarnation of this  commit was reviewed by RobH,
and if I understood him correctly he wanted the actual names of the
possible regulators to be used and specified in the bindings doc.   I
just followed the example of "pcie-rockchip-host.c" whose bindings doc
was reviewed by RobH.

Regards,
Jim Quinlan
Broadcom STB

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4167 bytes --]

  reply	other threads:[~2020-11-27 20:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-25 19:24 [PATCH v1 0/6] PCI: brcmstb: add EP regulators and panic handler Jim Quinlan
2020-11-25 19:24 ` [PATCH v1 1/6] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators Jim Quinlan
2020-11-25 19:24 ` [PATCH v1 2/6] PCI: brcmstb: Add control of EP voltage regulator(s) Jim Quinlan
2020-11-26 11:49   ` Mark Brown
2020-11-27 20:26     ` Jim Quinlan [this message]
2020-11-30 12:06       ` Mark Brown
2020-11-25 19:24 ` [PATCH v1 3/6] PCI: brcmstb: Do not turn off regulators if EP can wake up Jim Quinlan
2020-11-25 19:24 ` [PATCH v1 4/6] PCI: brcmstb: Give 7216 SOCs their own config type Jim Quinlan
2020-11-25 19:24 ` [PATCH v1 5/6] PCI: brcmstb: Add panic/die handler to RC driver Jim Quinlan
2020-11-25 19:24 ` [PATCH v1 6/6] PCI: brcmstb: check return value of clk_prepare_enable() Jim Quinlan

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=CA+-6iNzJAf_bKVjbw8bkh3qmSU++m6-DoFKQvBTTZGonYJGXfg@mail.gmail.com \
    --to=james.quinlan@broadcom.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=bhelgaas@google.com \
    --cc=broonie@kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=nsaenzjulienne@suse.de \
    --cc=robh@kernel.org \
    --subject='Re: [PATCH v1 2/6] PCI: brcmstb: Add control of EP voltage regulator(s)' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).