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?
> 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
