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

Hi Sebastian,

Thanks for the review.

> Your new binding is lacking too. If you select <&mipidcphy0 PHY_TYPE_CSI>
> you defined the direction of the PHY, but will it operate in C-PHY or in
> D-PHY mode?

You are right, and I am sorry - this is a regression I introduced in v3.

v2 already described these as two separate cells: the first selected the
electrical layer, and the second selected the direction. Michael Riesch
suggested using the generic PHY_TYPE_DSI/PHY_TYPE_CSI constants for the
second cell instead of the Rockchip-private RK_DCPHY_DIR_* constants [1],
and Vinod Koul agreed [2].

In v3, while making that change, I also mistakenly collapsed the two cells
into one. That is what lost the electrical-layer information you pointed
out.

For v4, I will go back to the v2 layout and only rename the second cell.
This keeps the two dimensions separate, with each cell describing one
thing:

/* MIPI DSI host - the transmitter */
phys = <&mipidcphy0 PHY_TYPE_DPHY PHY_TYPE_DSI>;

/* MIPI CSI-2 host - the receiver */
phys = <&mipidcphy0 PHY_TYPE_DPHY PHY_TYPE_CSI>;

The same applies to C-PHY, so the binding can express all four combinations
without any further changes:

phys = <&mipidcphy0 PHY_TYPE_CPHY PHY_TYPE_DSI>;
phys = <&mipidcphy0 PHY_TYPE_CPHY PHY_TYPE_CSI>;

The driver accepts all four combinations at phy_get() time and returns
-EOPNOTSUPP from power_on() for the C-PHY combinations, as it did before
this series. Boards that only wire up the transmitter keep
'#phy-cells = <1>', so the existing in-tree device trees remain unaffected.

Before sending v4, I will go through the code and commit messages again,
so that the reasoning above is captured in the commits themselves rather
than only in this thread. I will also re-test the series on the board.

Best regards,
Jason

[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/anSuxfeitSqmSHNr@vaman

Sebastian Reichel <[email protected]> 於 2026年8月13日週四 上午8:03寫道：
>
> Hi,
>
> On Wed, Aug 12, 2026 at 08:24:11PM +0800, 楊智成 wrote:
> > Thanks for the review.
> >
> > > I read above, but still do not get why TYPE_DPHY/CPHY is not enough.
> > > Isn't DPHY implying it is DSI?
> >
> > I see your point, and I did not explain this clearly enough in the previous
> > version.
> >
> > D-PHY only describes the electrical layer, and both MIPI DSI and MIPI CSI-2
> > can run on top of it. A CSI-2 receiver's PHY is a D-PHY just as much as a
> > DSI transmitter's is, so PHY_TYPE_DPHY alone does not tell us which one the
> > consumer is asking for.
> >
> > That is the problem here. The RK3588 DC-PHY exposes both a transmitter and
> > a receiver from a single PHY block, which can be used by two independent
> > consumers at the same time. This is not a theoretical concern: on this
> > board a DSI panel is scanning out while the same PHY receives CSI-2 frames
> > from a camera. With only the electrical layer to identify the PHY, both
> > consumers would end up with the same phandle cell:
> >
> > dsi@fde20000 {
> > phys = <&mipidcphy0 PHY_TYPE_DPHY>; /* wants the TX */
> > };
> >
> > csi2@fdd10000 {
> > phys = <&mipidcphy0 PHY_TYPE_DPHY>; /* wants the RX */
> > };
> >
> > There is then nothing in .of_xlate() to distinguish the two requests.
> >
> > I will make this clearer in the v4 commit message and include the example
> > above so that the reasoning is easier to follow.
> >
> > For context, v2 described the direction with a Rockchip-private
> > RK_DCPHY_DIR_* enum. Michael Riesch suggested using generic constants
> > instead [1], and Vinod agreed [2].
> >
> > [1] https://lore.kernel.org/r/[email protected]
> > [2] https://lore.kernel.org/r/anSuxfeitSqmSHNr@vaman
>
> Your new binding is lacking too. If you select <&mipidcphy0 PHY_TYPE_CSI>
> you defined the direction of the PHY, but will it operate in C-PHY or in
> D-PHY mode?
>
> Greetings,
>
> -- Sebastian
>
> > > Your tag goes the last.
> >
> > Sure, I will fix this in v4. The Signed-off-by tag will come last, and I
> > will check the whole series again.
> >
> > > Two simple defines needed Claude.
> >
> > Yes, I agree that these two defines themselves are simple and do not really
> > need AI assistance.
> >
> > I added the Assisted-by tag because I used AI during the development of the
> > series as a whole, including cross-checking the code, writing additional
> > test cases, and looking up the relevant sections of the TRM. (I checked the
> > corresponding sections in the TRM myself, reviewed the test cases, and
> > re-ran them on the hardware before sending the series.)
> >
> > I also checked the current mainline guidance in
> > Documentation/process/submitting-patches.rst and
> > Documentation/process/coding-assistants.rst. Since I was not sure how much
> > AI involvement should warrant tagging individual patches, I chose to mark
> > the whole series consistently.
> >
> > That said, I am happy to drop the tag from this patch in v4 if you prefer -
> > I wrote these two lines myself.
> >
> > Thanks,
> > Jason
> >
> >
> > Krzysztof Kozlowski <[email protected]> 於 2026年8月12日週三 下午6:51寫道：
> > >
> > > On Mon, Aug 10, 2026 at 08:10:09PM +0800, Jason Yang wrote:
> > > > MIPI D-PHY and C-PHY blocks are increasingly direction-agnostic: the
> > > > same PHY IP can drive a MIPI DSI display or receive from a MIPI CSI-2
> > > > camera, and combo blocks like the Samsung IP on RK3588 expose both
> > > > directions to independent consumers at the same time. A binding that
> > > > needs to tell the two consumers apart has nothing generic to reach
> > > > for: most constants in this header name a protocol (PHY_TYPE_USB3,
> > > > PHY_TYPE_DP, ...), while the MIPI entries name only the electrical
> > > > layer.
> > > >
> > > > Add PHY_TYPE_DSI and PHY_TYPE_CSI to select a PHY by the MIPI
> > > > protocol it speaks, which also implies the direction. They do not
> > > > replace PHY_TYPE_DPHY/PHY_TYPE_CPHY, which remain the right choice
> > > > where the cell selects the electrical layer. First user is the
> > > > Rockchip RK3588 MIPI DC-PHY binding.
> > >
> > > I read above, but still do not get why TYPE_DPHY/CPHY is not enough.
> > > Isn't DPHY implying it is DSI?
> > >
> > > >
> > > > Suggested-by: Michael Riesch <[email protected]>
> > > > Signed-off-by: Jason Yang <[email protected]>
> > >
> > > Your tag goes the last.
> > >
> > > > Assisted-by: Claude:claude-fable-5
> > >
> > > Two simple defines needed Claude. Great, that probably makes AI
> > > conglomerates very happy that we do not type even two lines anymore and
> > > need their resource-hungry data centers to do that for us.
> > >
> > > Best regards,
> > > Krzysztof
> > >
> >

-- 
linux-phy mailing list
[email protected]
https://lists.infradead.org/mailman/listinfo/linux-phy