Re: [PATCH v2 1/5] dt-bindings: PCI: Add STM32MP25 PCIe root complex bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/17/24 18:25, Manivannan Sadhasivam wrote:
On Tue, Dec 17, 2024 at 04:53:48PM +0100, Christian Bruel wrote:

Makes sense.  What about phys, resets, etc?  I'm pretty sure a PHY
would be a per-Root Port thing, and some resets and wakeup signals
also.

For new drivers, I think we should start adding Root Port stanzas to
specifically associate those things with the Root Port, e.g.,
something like this?

    pcie@48400000 {
      compatible = "st,stm32mp25-pcie-rc";

      pcie@0,0 {
        reg = <0x0000 0 0 0 0>;
        phys = <&combophy PHY_TYPE_PCIE>;
        phy-names = "pcie-phy";
      };
    };

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml?id=v6.12#n111
is one binding that does this, others include apple,pcie.yaml,
brcm,stb-pcie.yaml, hisilicon,kirin-pcie.yaml.


On a second thought, moving the PHY to the root-port part would introduce a
discrepancy with the pcie_ep binding, whereas the PHY is required on the
pcie_ep node.

Even for the pcie_rc, the PHY is needed to enable the core_clk to access
the PCIe core registers,


But why that matters? You can still parse the child nodes, enable PHY and
configure PCIe registers. >
So that would make 2 different required PHY locations for RC and EP:

     pcie_rc: pcie@48400000 {
       compatible = "st,stm32mp25-pcie-rc";

       pcie@0,0 {
         reg = <0x0000 0 0 0 0>;
         phys = <&combophy PHY_TYPE_PCIE>;
         phy-names = "pcie-phy";
       };
     };

     pcie_ep pcie@48400000 {
       compatible = "st,stm32mp25-pcie-ep";
       phys = <&combophy PHY_TYPE_PCIE>;
       phy-names = "pcie-phy";
     };

Simplest seems to keep the PHY required for the pcie core regardless of the
mode and keep the empty root port to split the design


No please. Try to do the right thing from the start itself.

Parsing the child node to clock the IP seems weird. Note that hisilicon,kirin-pcie.yaml also declares the PHY at the controller level.

thanks

Christian





- Mani





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux