Re: [PATCH v4 1/7] dt-bindings: phy: Add PHY_TYPE_DSI and PHY_TYPE_CSI definitions

[Krzysztof Kozlowski <krzk@kernel.org>]

On 28/08/2026 11:35, 楊智成 wrote:
> Hi Krzysztof,
> 
> Thanks for the review.
> 
> (from the review of 2/7)
> 
>> Please link/show the upstream DTS using simultaneously transmitter and
>> receiver, where the consumer does not know with whom is it talking to.
> 
> I grepped the tree and found a device tree that has simultaneously
> transmitter and receiver. In the arch/arm/boot/dts/samsung/*,
> exynos4412-midas.dtsi enables csis_0, and exynos4412-galaxy-s3.dtsi, which
> includes it, enables dsi_0. Both consumers are declared in exynos4.dtsi,
> on the same phy node:
> 
>     dsi_0:  phys = <&mipi_phy 1>;
>     csis_0: phys = <&mipi_phy 0>;
> 
> Tracing that back, the provider is
> Documentation/devicetree/bindings/phy/samsung,mipi-video-phy.yaml and
> drivers/phy/samsung/phy-exynos-mipi-video.c.
> It registers several struct phy in one node, and the binding says:

Please do not bring 15 year old code as example how this should or could
be done.

And in Samsung - what makes you thinking this is the same phy? It's just
way how it was represented in PMU, which is controlling power of entire
SoC, including MIPI PHY0 and PHY1, even though there is only one device
node in the code. Why taking some random anti-pattern into discussion
here, really...

I want to see your upstream DTS code.


> 
>       For samsung,s5pv210-mipi-video-phy compatible PHYs the second cell in the
>       PHY specifier identifies the PHY and its meaning is as follows::
>         0 - MIPI CSIS 0,
>         1 - MIPI DSIM 0,
>         2 - MIPI CSIS 1,
>         3 - MIPI DSIM 1.
> 
> Resuming from the cover letter in v4, I treat this phy as one tx phy and one
> rx phy. Maybe I can follow this pattern and introduce phy 0 and phy 1 in
> rockchip,rk3588-mipi-dcphy.yaml, and add an enum for
> SAMSUNG_MIPI_DCPHY_TX/RX in the driver. The binding would then say:

But why is this one phy? Describe your hardware, show some diagrams from
user manual etc.


> 
>       Two cells select the mode and then one of the two PHYs that the
>       block contains:
>         0 - the transmitter, for a MIPI DSI host
>         1 - the receiver, for a MIPI CSI-2 host
> 
>       A single cell is equivalent to a second cell of 0.
> 
> In this way, I don't need to add PHY_TYPE_CSI/DSI to
> include/dt-bindings/phy/phy.h.
> And I think this will not go against the discussion in v2, where Michael pointed
> out that DSI and CSI are standard protocols and should not be named in
> a Rockchip
> specific header, and suggested introducing PHY_TYPE_CSI/DSI:
> 
>> How is that Rockchip specific? As you state above this is either MIPI
>> CSI or MIPI DSI, two perfectly standard protocols.
>>
>> @all Would it be reasonable to introduce PHY_TYPE_CSI and PHY_TYPE_DSI?
> 
> https://lore.kernel.org/r/82da3622-9c3a-454c-87bc-fb4ec7adb68d@collabora.com
> 
>> Where is any DTS using it? I did a search and no results of upstream
>> posting.
> 
> The user will be rk3588-base.dtsi. It adds csi0/1, which connect back
> to the existing vicap_mipi0/1 ports. It also sets #phy-cells

"Will" does not work that much because I doubt correctness of your
design and that DTS might never be sent upstream.

Post your complete upstream DTS.


Best regards,
Krzysztof

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip