devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vitor Soares <ivitro@gmail.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Vitor Soares" <vitor.soares@toradex.com>,
	linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies
Date: Mon, 27 Oct 2025 23:22:26 +0000	[thread overview]
Message-ID: <2c3e4bdefb306dc89c15bebc549d854ea2b4cc32.camel@gmail.com> (raw)
In-Reply-To: <20251020-kickass-fervent-capybara-9c48a0@kuoka>

Hi Krzysztof,

Thank you for the feedback.

On Mon, 2025-10-20 at 13:14 +0200, Krzysztof Kozlowski wrote:
> On Tue, Oct 14, 2025 at 12:25:48PM +0100, Vitor Soares wrote:
> > From: Vitor Soares <vitor.soares@toradex.com>
> > 
> > Add optional regulator supply properties for PCIe endpoints on TI SoCs.
> > Some boards provide dedicated regulators for PCIe devices, such as
> > 1.5V (miniPCIe), 3.3V (common for M.2 or miniPCIe), or 12V
> > (for high-power devices). These supplies are now described as optional
> > properties to allow the driver to control endpoint power where supported.
> 
> Last sentence is completely redundant. Please do not describe DT, we
> all can read the patch. Driver is irrelevant here.
> 
> 
Ack, I will remove last sentence.

> 
> How you described here and in descriptions, suggests these are rather
> port properties, not the controller.

You are right - these supplies power the PCIe slot/connector, not the controller
itself. However, as per my understanding, the current kernel practice is to
place slot supplies in the root complex node rather than the endpoint node. as
seen in e.g.:
- imx6q-pcie.yaml
- rockchip-dw-pcie.yaml
- rcar-pci-host.yaml

This seems consistent with those existing bindings, but please let me know if
I’m overlooking something specific to this case.

> 
> > 
> > Signed-off-by: Vitor Soares <vitor.soares@toradex.com>
> > ---
> >  .../devicetree/bindings/pci/ti,j721e-pci-host.yaml | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > index c704099f134b..a20b03406448 100644
> > --- a/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > +++ b/Documentation/devicetree/bindings/pci/ti,j721e-pci-host.yaml
> > @@ -110,6 +110,18 @@ properties:
> >        interrupts:
> >          maxItems: 1
> >  
> > +  vpcie1v5-supply:
> 
> How is it called in this device datasheet (not the board schematics)?

The TI SoC datasheet describes the controller interface but doesn’t define these
external supply rails - they are board-level regulators specific to the slot.

> 
> > +    description: 1.5V regulator used to power PCIe interfaces,
> > +                 typically present on miniPCIe slots.
> 
> Best regards,
> Krzysztof
> 

Best regards,
Vitor Soares

  reply	other threads:[~2025-10-27 23:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-14 11:25 [PATCH v1 0/2] PCI: j721e: Add voltage regulator support Vitor Soares
2025-10-14 11:25 ` [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add optional regulator supplies Vitor Soares
2025-10-20 11:14   ` Krzysztof Kozlowski
2025-10-27 23:22     ` Vitor Soares [this message]
2025-10-28  5:41       ` Manivannan Sadhasivam
2025-10-28 23:35         ` Vitor Soares

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=2c3e4bdefb306dc89c15bebc549d854ea2b4cc32.camel@gmail.com \
    --to=ivitro@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kishon@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=robh@kernel.org \
    --cc=vitor.soares@toradex.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).