RE: [PATCH v2 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC

[Shradha Todi]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Sent: 01 July 2025 16:55
> To: Shradha Todi <shradha.t@xxxxxxxxxxx>; 'Rob Herring' <robh@xxxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> samsung-soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-phy@xxxxxxxxxxxxxxxxxxx; linux-
> fsd@xxxxxxxxx; mani@xxxxxxxxxx; lpieralisi@xxxxxxxxxx; kw@xxxxxxxxx; bhelgaas@xxxxxxxxxx;
> jingoohan1@xxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx;
> vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; arnd@xxxxxxxx; m.szyprowski@xxxxxxxxxxx;
> jh80.chung@xxxxxxxxxxx; pankaj.dubey@xxxxxxxxxxx
> Subject: Re: [PATCH v2 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC
> 
> On 01/07/2025 13:06, Shradha Todi wrote:
> >
> >
> >> -----Original Message-----
> >> From: Rob Herring <robh@xxxxxxxxxx>
> >> Sent: 28 June 2025 02:47
> >> To: Shradha Todi <shradha.t@xxxxxxxxxxx>
> >> Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > linux-
> >> samsung-soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-phy@xxxxxxxxxxxxxxxxxxx; linux-
> >> fsd@xxxxxxxxx; manivannan.sadhasivam@xxxxxxxxxx; lpieralisi@xxxxxxxxxx; kw@xxxxxxxxx;
> >> bhelgaas@xxxxxxxxxx; jingoohan1@xxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> >> alim.akhtar@xxxxxxxxxxx; vkoul@xxxxxxxxxx; kishon@xxxxxxxxxx; arnd@xxxxxxxx;
> >> m.szyprowski@xxxxxxxxxxx; jh80.chung@xxxxxxxxxxx; pankaj.dubey@xxxxxxxxxxx
> >> Subject: Re: [PATCH v2 07/10] dt-bindings: phy: Add PHY bindings support for FSD SoC
> >>
> >> On Wed, Jun 25, 2025 at 10:22:26PM +0530, Shradha Todi wrote:
> >>> Document PHY device tree bindings for Tesla FSD SoCs.
> >>>
> >>> Signed-off-by: Shradha Todi <shradha.t@xxxxxxxxxxx>
> >>> ---
> >>>  .../bindings/phy/samsung,exynos-pcie-phy.yaml | 25 +++++++++++++++++--
> >>>  1 file changed, 23 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
> >> b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
> >>> index 41df8bb08ff7..4dc20156cdde 100644
> >>> --- a/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
> >>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos-pcie-phy.yaml
> >>> @@ -15,10 +15,13 @@ properties:
> >>>      const: 0
> >>>
> >>>    compatible:
> >>> -    const: samsung,exynos5433-pcie-phy
> >>> +    enum:
> >>> +      - samsung,exynos5433-pcie-phy
> >>> +      - tesla,fsd-pcie-phy
> >>>
> >>>    reg:
> >>> -    maxItems: 1
> >>> +    minItems: 1
> >>> +    maxItems: 2
> >>>
> >>>    samsung,pmu-syscon:
> >>>      $ref: /schemas/types.yaml#/definitions/phandle
> >>> @@ -30,6 +33,24 @@ properties:
> >>>      description: phandle for FSYS sysreg interface, used to control
> >>>                   sysreg registers bits for PCIe PHY
> >>>
> >>> +allOf:
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - tesla,fsd-pcie-phy
> >>> +    then:
> >>> +      description:
> >>> +        The PHY controller nodes are represented in the aliases node
> >>> +        using the following format 'pciephy{n}'. Depending on whether
> >>> +        n is 0 or 1, the phy init sequence is chosen.
> >>
> >> What? Don't make up your own aliases.
> >>
> >> If the PHY instances are different, then maybe you need a different
> >> compatible. If this is just selecting the PHY mode, you can do that in
> >> PHY cells as the mode depends on the consumer.
> >>
> >
> > FSD PCIe has 2 instances of PHY. Both are the same HW Samsung
> > PHYs (Therefore share the same register offsets). But the PHY used here
> 
> So same?
> 
> > does not support auto adaptation so we need to tune the PHYs
> > according to the use case (considering channel loss, etc). This is why we
> 
> So not same? Decide. Either it is same or not, cannot be both.
> 
> If you mean that some wiring is different on the board, then how does it
> differ in soc thus how it is per-soc property? If these are use-cases,
> then how is even suitable for DT?
> 
> I use your Tesla FSD differently and then I exchange DTSI and compatibles?
> 
> You are no describing real problem and both binding and your
> explanations are vague and imprecise. Binding tells nothing about it, so
> it is example of skipping important decisions.
> 
> > have 2 different SW PHY initialization sequence depending on the instance
> > number. Do you think having different compatible (something like
> > tesla,fsd-pcie-phy0 and tesla,fsd-pcie-phy1) and having phy ID as platform data
> > is okay in this case? I actually took reference from files like:
> 
> And in different use case on same soc you are going to reverse
> compatibles or instance IDs?
>

Even though both the PHYs are exactly identical in terms of hardware,
they need to be programmed/initialized/configured differently.

Sorry for my misuse of the word "use-case". To clarify, these configurations
will always remain the same for FSD SoC even if you use it differently.

I will use different compatibles for them as I understand that it is the best
option.
 
> > drivers/usb/phy/phy-am335x-control.c
> 
> So you took 15 years old hardware, code and binding as an example.
> 
> No, don't do that ever.
> 
> Anyway, poor choices even in newer code should not drive your design.
> Design it properly, describe the hardware.
> 
> > drivers/phy/freescale/phy-fsl-imx8-mipi-dphy.c
> > who use alias to differentiate between register offsets for instances.
> 
> 
> 
> Best regards,
> Krzysztof