From: "Pali Rohár" <pali@kernel.org>
To: Jim Quinlan <james.quinlan@broadcom.com>
Cc: Rob Herring <robh@kernel.org>, Jim Quinlan <jim2101024@gmail.com>,
PCI <linux-pci@vger.kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
Nicolas Saenz Julienne <nsaenz@kernel.org>,
Mark Brown <broonie@kernel.org>,
"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE"
<bcm-kernel-feedback-list@broadcom.com>,
Sean V Kelley <sean.v.kelley@intel.com>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>,
Qiuxu Zhuo <qiuxu.zhuo@intel.com>,
Keith Busch <kbusch@kernel.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v8 5/8] PCI/portdrv: add mechanism to turn on subdev regulators
Date: Thu, 18 Nov 2021 16:50:38 +0100 [thread overview]
Message-ID: <20211118155038.3x4bwgubbnuxv3dy@pali> (raw)
In-Reply-To: <CA+-6iNwUORmdLy9ii748K4JfZQ8J-N48r-q7QO1P9XAZR2W2qw@mail.gmail.com>
On Thursday 18 November 2021 10:36:00 Jim Quinlan wrote:
> On Wed, Nov 17, 2021 at 10:45 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Wednesday 17 November 2021 10:14:19 Jim Quinlan wrote:
> > > On Tue, Nov 16, 2021 at 3:53 PM Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > Yes, I was looking at it... main power (12V/3.3V) and AUX power (3.3V)
> > > > needs to be supplied at the "correct" time during establishing link
> > > > procedure. I wrote it in my RFC email:
> > > > https://lore.kernel.org/linux-pci/20211022183808.jdeo7vntnagqkg7g@pali/
> > > Hello Pali,
> > >
> > > I really like your proposal although I would like to get my patchset
> > > first :-) :-)
> > >
> > > Suppose you came up with a patchset for your ideas-- would that include
> > > changes to existing RC drivers to use the proposed framework? If so,
> > > I am wary that it would
> > > break at least a few of them. Or would you just present the framework
> > > and allow the
> > > RC drivers' authors to opt-in, one by one?
> >
> > My idea is to add new "framework" to allow drivers implement new
> > callbacks for this "framework". There would be no change in drivers
> > which do not provide these callbacks to ensure that nothing is going to
> > be broken. I'm planning to implement these callbacks only for RC drivers
> > for which I have hardware and can properly test to not introduce any
> > regression. For other existing RC drivers it is up to other authors +
> > testers. But to decrease future maintenance cost of all RC drivers I
> > expect that new drivers would not implement any ad-hoc solution in their
> > "probe" function and instead implement these new callbacks. That is my
> > idea.
> >
> > > At any rate, if you want someone to test some of your ideas I can work
> > > with you.
> >
> > Perfect! If you have any concerns or you see any issues, please reply
> > that my RFC email. So I can collect feedback.
> >
> > Also I sent draft for updating DTS schema for PCIe devices:
> > https://github.com/devicetree-org/dt-schema/pull/64
>
> Hi Pali,
> I don't see any mention or placement of the regulator nodes for power;
I put in above pull request draft only existing attributes (from
pci.txt), I have not introduce anything new yet.
> do you agree with where
> I proposed we place them, ie in the first bridge under the root-complex, e.g.
>
> pcie0: pcie@7d500000 { /*
> root complex */
> compatible = "brcm,bcm2711-pcie";
> reg = <0x0 0x7d500000 0x9310>;
>
> /* PCIe bridge */
> pci@0,0 {
> #address-cells = <3>;
> #size-cells = <2>;
> reg = <0x0 0x0 0x0 0x0 0x0>;
> compatible = "pciclass,0604";
> device_type = "pci";
> vpcie3v3-supply = <&vreg7>; /*
> <------------- HERE */
This node 'pci@0,0' describes PCIe Root Port. So yes, it is place where
power regulators belongs. I agree with you.
(Note: I would suggest to use /* PCIe Root Port */ comment instead of
/* PCIe bridge */. As PCIe bridge is ambiguous name which could mean
also other devices.)
> ranges;
>
> pci-ep@0,0 { /* PCIe endpoint */
> assigned-addresses =
> <0x82010000 0x0 0xf8000000 0x6
> 0x00000000 0x0 0x2000>;
> reg = <0x0 0x0 0x0 0x0 0x0>;
> compatible = "pci14e4,1688";
> #address-cells = <3>;
> #size-cells = <2>;
>
> ranges;
> };
> };
> };
>
>
> Regards,
> Jim
next prev parent reply other threads:[~2021-11-18 15:50 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-10 22:14 [PATCH v8 0/8] PCI: brcmstb: have portdrv turn on sub-device power Jim Quinlan
2021-11-10 22:14 ` [PATCH v8 1/8] PCI: brcmstb: Change brcm_phy_stop() to return void Jim Quinlan
2021-11-11 21:57 ` Bjorn Helgaas
2021-11-15 20:56 ` Jim Quinlan
2021-11-16 20:40 ` Bjorn Helgaas
2021-11-10 22:14 ` [PATCH v8 2/8] dt-bindings: PCI: Correct brcmstb interrupts, interrupt-map Jim Quinlan
2021-11-10 22:14 ` [PATCH v8 3/8] dt-bindings: PCI: Add bindings for Brcmstb EP voltage regulators Jim Quinlan
2021-11-11 22:17 ` Bjorn Helgaas
2021-11-12 18:25 ` Jim Quinlan
2021-11-12 20:20 ` Bjorn Helgaas
2021-11-12 21:46 ` Rob Herring
2021-11-13 11:38 ` Pali Rohár
2021-11-10 22:14 ` [PATCH v8 4/8] PCI/portdrv: Create pcie_is_port_dev() func from existing code Jim Quinlan
2021-11-11 21:51 ` Florian Fainelli
2021-11-11 22:53 ` Rob Herring
2021-11-11 23:50 ` Krzysztof Wilczyński
2021-11-12 18:14 ` Jim Quinlan
2021-11-10 22:14 ` [PATCH v8 5/8] PCI/portdrv: add mechanism to turn on subdev regulators Jim Quinlan
2021-11-11 9:44 ` kernel test robot
2021-11-11 22:12 ` Bjorn Helgaas
2021-11-11 22:50 ` Rob Herring
2021-11-11 22:56 ` Rob Herring
2021-11-15 20:44 ` Jim Quinlan
2021-11-16 17:41 ` Rob Herring
2021-11-16 20:53 ` Pali Rohár
2021-11-17 15:14 ` Jim Quinlan
2021-11-17 15:45 ` Pali Rohár
2021-11-18 15:36 ` Jim Quinlan
2021-11-18 15:50 ` Pali Rohár [this message]
2021-11-17 14:46 ` Jim Quinlan
2021-11-11 23:38 ` Krzysztof Wilczyński
2021-11-15 20:26 ` Jim Quinlan
2021-11-10 22:14 ` [PATCH v8 6/8] PCI/portdrv: Do not turn off subdev regulators if EP can wake up Jim Quinlan
2021-11-10 22:14 ` [PATCH v8 7/8] PCI: brcmstb: Split brcm_pcie_setup() into two funcs Jim Quinlan
2021-11-10 22:14 ` [PATCH v8 8/8] PCI: brcmstb: Add control of subdevice voltage regulators 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=20211118155038.3x4bwgubbnuxv3dy@pali \
--to=pali@kernel.org \
--cc=Jonathan.Cameron@huawei.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=bhelgaas@google.com \
--cc=broonie@kernel.org \
--cc=james.quinlan@broadcom.com \
--cc=jim2101024@gmail.com \
--cc=kbusch@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=nsaenz@kernel.org \
--cc=qiuxu.zhuo@intel.com \
--cc=robh@kernel.org \
--cc=sean.v.kelley@intel.com \
/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 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).