Devicetree
 help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: "Heiko Stübner" <heiko@sntech.de>
Cc: 楊智成 <jason98166@gmail.com>,
	"Michael Riesch" <michael.riesch@collabora.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Manivannan Sadhasivam" <mani@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Guochun Huang" <hero.huang@rock-chips.com>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Sebastian Reichel" <sebastian.reichel@collabora.com>,
	"Bryan O'Donoghue" <bryan.odonoghue@linaro.org>,
	linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/8] dt-bindings: phy: rockchip,rk3588-mipi-dcphy: Add a cell to select TX or RX
Date: Tue, 20 Oct 2026 09:10:39 -0500	[thread overview]
Message-ID: <20261020141039.GB4157247-robh@kernel.org> (raw)
In-Reply-To: <2501273.NG923GbCHz@diego>

On Tue, Oct 20, 2026 at 03:37:35PM +0200, Heiko Stübner wrote:
> Am Dienstag, 20. Oktober 2026, 00:20:47 Mitteleuropäische Sommerzeit schrieb Michael Riesch:
> > Hi all,
> > 
> > On 10/19/26 17:06, Rob Herring wrote:
> > > On Mon, Oct 19, 2026 at 04:15:03PM +0800, 楊智成 wrote:
> > >> Hi Krzysztof, Rob,
> > >>
> > >>> Existing OS should ignore surplus cell if the driver is coded properly.
> > >>> Same as adding new properties or new front-compatible.  The question is
> > >>> how the existing driver behaves?
> > >>
> > >> As you pointed out on 5/8, the xlate checks args_count != 1. The DC-PHY
> > >> driver was merged in v6.15, so every kernel since 6.15 fails on a second cell.
> > >>
> > >> 1. Keep the second cell. Add the patch as follows:
> > >>
> > >> - if (args->args_count != 1) {
> > >> + if (args->args_count < 1) {
> > >>     dev_err(dev, "invalid number of arguments\n");
> > >>     return ERR_PTR(-EINVAL);
> > >>   }
> > >>
> > >> + if (args->args_count > 1 && args->args[1]) {
> > >> +   dev_err(dev, "only the transmitter is supported\n");
> > >> +   return ERR_PTR(-EINVAL);
> > >> + }
> > >>
> > >>   With Fixes: and Cc: stable, the stable kernels maintained at that
> > >>   time (currently 6.18.y and 7.2.y) keep DSI working with the new
> > >>   dts, and the receiver fails cleanly instead of getting the transmitter.
> > >>   But EOL kernels (6.15, 6.16, 6.17, 6.19, 7.0, 7.1) stay affected.
> > >>   It goes in with the series, since two-cell dts only exist after it.
> > >>
> > >> 2. Use a flag in the upper bits of the existing cell, as Rob
> > >>     suggested. Existing kernels are not affected, unless a board enables
> > >>     the DC-PHY CSI-2 receiver on 7.0 or later without this series.
> > >>
> > >> I will write the impact clearly whichever  approach is selected in
> > >> the related commit messages and the cover letter.
> > >>
> > >> I think the cell approach is easier to read and more symmetric in the
> > >> long term,
> > >> but it breaks more. Which one do you think is better? Thanks.
> > > 
> > > I'm not a user of this platform, so I don't care. Up to the platform 
> > > maintainers to decide if breakage is acceptible. You just need to 
> > > document the issue in the commit message.
> > > 
> > > Rob
> > 
> > It's a bit of a pickle, all this.
> > 
> > I gave it some more thought. In the end this is an IP block with two
> > PHYs. No more, no less. One cell is sufficient to address those, and it
> > could be 0 for the transmitting PHY (DSI, display) and 1 for the
> > receiving PHY (CSI, camera).
> > 
> > Of course the two PHYs can operate in either DPHY or CPHY mode. The
> > phy-cells have been (mis?)used to select the proper mode of combo PHYs,
> > and this principle has been applied here too.
> > 
> > However, we have the special situation here that the mode can only be
> > changed for the complete IP block (and thus for both PHYs at the same
> > time). As Bryan points out, we need to check and complain if the PHY
> > mode is not set to the same value for both PHYs.
> > 
> > I suggest using a property
> >     samsung,cphy-mode;
> 
> I guess, a maybe nicer way would be mimicing
> 	Documentation/devicetree/bindings/phy/intel,combo-phy.yaml
> 
> with its
> 	intel,phy-mode = <x>
> 
> with x coming from include/dt-bindings/phy/phy.h
> for the distinction between cphy and dphy, so we don't introduce yet
> another way to describe this.

We already have the plain 'phy-type' property which takes the same 
PHY_MODE_x values. We don't need yet another vendor specific property.

> 
> > (name to be bikeshedded) that sets the complete IP block into CPHY mode
> > (DPHY being the default) to distinguish between the modes. Since both
> > modes use a different electrical interface, this is clearly a hardware
> > thing and thus suitable for device tree.
> > 
> > Then, we can address the two PHYs with the help of the already existing
> > phy cell (without any bitflag tweaks to the value).
> > 
> > The sole drawback is that we need to accept the PHY_TYPE_DPHY and
> > PHY_TYPE_CPHY values that some OS in the field might pass. (For Linux,
> > it should only be the former.) And we need to handle those values
> > correctly. But I think this is not more painful than the other
> > approaches. At the same time, this design seems to be the most correct
> 
> The current cell values are 10 + 11, so as long as we don't re-use those
> exact values for the other constants the driver should somewhat easily
> be able to recognize an old DT?

I think you will still have an ABI issue of what does old OS do with new 
values?

Rob

  reply	other threads:[~2026-10-20 14:10 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16  8:56 [PATCH v5 0/8] phy: rockchip-samsung-dcphy: Add the MIPI D-PHY receiver Jason Yang
2026-10-16  8:56 ` [PATCH v5 1/8] dt-bindings: phy: Add PHY_DIRECTION_TX/RX definitions Jason Yang
2026-10-16 10:38   ` Bryan O'Donoghue
2026-10-17 17:19   ` Krzysztof Kozlowski
2026-10-16  8:56 ` [PATCH v5 2/8] dt-bindings: phy: rockchip,rk3588-mipi-dcphy: Add a cell to select TX or RX Jason Yang
2026-10-16 10:43   ` Bryan O'Donoghue
2026-10-19  8:51     ` 楊智成
2026-10-16 14:01   ` Rob Herring
2026-10-17 17:22     ` Krzysztof Kozlowski
2026-10-19  8:15       ` 楊智成
2026-10-19 15:06         ` Rob Herring
2026-10-19 22:20           ` Michael Riesch
2026-10-20 13:37             ` Heiko Stübner
2026-10-20 14:10               ` Rob Herring [this message]
2026-10-20 14:57                 ` Heiko Stübner
2026-10-20 18:04                   ` Michael Riesch
2026-10-20 18:24                     ` Heiko Stübner
2026-10-16  8:56 ` [PATCH v5 3/8] phy: rockchip-samsung-dcphy: Move block-level setup to runtime resume Jason Yang
2026-10-16  9:09   ` sashiko-bot
2026-10-16 10:46   ` Bryan O'Donoghue
2026-10-16  8:56 ` [PATCH v5 4/8] phy: rockchip-samsung-dcphy: Name the transmitter helpers and ops Jason Yang
2026-10-16  9:07   ` sashiko-bot
2026-10-16  8:56 ` [PATCH v5 5/8] phy: rockchip-samsung-dcphy: Add a second PHY for the receiver Jason Yang
2026-10-17 17:24   ` Krzysztof Kozlowski
2026-10-16  8:56 ` [PATCH v5 6/8] phy: rockchip-samsung-dcphy: Add MIPI D-PHY receiver support Jason Yang
2026-10-16  8:56 ` [PATCH v5 7/8] arm64: dts: rockchip: use two phy cells for the rk3588 mipi dcphys Jason Yang
2026-10-17 17:37   ` Krzysztof Kozlowski
2026-10-16  8:56 ` [PATCH v5 8/8] arm64: dts: rockchip: add mipi csi-2 receiver nodes for the dcphys on rk3588 Jason Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261020141039.GB4157247-robh@kernel.org \
    --to=robh@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=heiko@sntech.de \
    --cc=hero.huang@rock-chips.com \
    --cc=jason98166@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mani@kernel.org \
    --cc=michael.riesch@collabora.com \
    --cc=neil.armstrong@linaro.org \
    --cc=p.zabel@pengutronix.de \
    --cc=sebastian.reichel@collabora.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox