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/82da3622-9c3a-454c-87bc-fb4ec7adb68d@collabora.com (local)
[2] https://lore.kernel.org/r/anSuxfeitSqmSHNr@vaman (local)
Sebastian Reichel [off-list ref] 於 2026年8月13日週四 上午8:03寫道：
Hi,
On Wed, Aug 12, 2026 at 08:24:11PM +0800, 楊智成 wrote:
quoted
Thanks for the review.
quoted
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/82da3622-9c3a-454c-87bc-fb4ec7adb68d@collabora.com (local)
[2] https://lore.kernel.org/r/anSuxfeitSqmSHNr@vaman (local)
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
quoted
quoted
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.
quoted
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 [off-list ref] 於 2026年8月12日週三 下午6:51寫道：
quoted
On Mon, Aug 10, 2026 at 08:10:09PM +0800, Jason Yang wrote:
quoted
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?
quoted
Suggested-by: Michael Riesch <michael.riesch@collabora.com>
Signed-off-by: Jason Yang <redacted>
Your tag goes the last.
quoted
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