Message-ID: <f99c528a19fa793035cece3f83b332d7ecafa7da.camel@gmail.com>
Date: Tue, 28 Oct 2025 23:35:48 +0000
From: Vitor Soares <ivitro@...il.com>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Krzysztof Kozlowski <krzk@...nel.org>, Bjorn Helgaas
 <bhelgaas@...gle.com>,  Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof Wilczyński <kwilczynski@...nel.org>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>, Vitor
 Soares <vitor.soares@...adex.com>, linux-pci@...r.kernel.org, 
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: PCI: ti,j721e-pci-host: Add
 optional regulator supplies

On Tue, 2025-10-28 at 11:11 +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 27, 2025 at 11:22:26PM +0000, Vitor Soares wrote:
> > 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@...adex.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.
> > 
> 
> We do not properly document it, but defining the slot supplies in host bridge
> (controller) node is deprecated. Some bindings still do it for legacy reasons,
> but the new ones should define them in the Root Port nodes as they belong to.
> We
> do not have a separate DT node for PCI slots, but rather reuse the Root Port
> node.
> 
> There are also bindings that define supplies in the endpoint node. They do it
> for devices directly connected to the PCI bus without a connector (like in
> PCB).
> 
> - Mani
> 

Thanks for the clarification and context. From what I understand, the
recommendation is to define the supply regulators under the individual root port
node rather than in the host bridge (controller) node, as the supplies
conceptually belong to each port rather than the controller itself.

On the j721e PCIe controller, the current driver implementation assumes a single
root port and doesn’t parse child port nodes. To follow the new convention, I’d
need to refactor the driver to support root port subnodes, and I wonder if the
PHY reference and reset should also be moved to the Root Port node in that case.

Could you please point me to an example of a PCIe controller binding or driver
that already follows this approach?

Best regards,
Vitor Soares

Powered by blists - more mailing lists