Re: [PATCH v5 2/8] dt-bindings: phy: rockchip,rk3588-mipi-dcphy: Add a cell to select TX or RX

["Heiko Stübner" <heiko@sntech.de>]

Hi Michael,

Am Dienstag, 20. Oktober 2026, 20:04:33 Mitteleuropäische Sommerzeit schrieb Michael Riesch:
> On 10/20/26 16:57, Heiko Stübner wrote:
> > Am Dienstag, 20. Oktober 2026, 16:10:39 Mitteleuropäische Sommerzeit schrieb Rob Herring:
> >> 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.
> > 
> > even better then ... and I didn't look deep enough it seems.
> > 
> > 
> >>>> (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?
> > 
> > wasn't the main contract, a newer kernel should support old DTs ?
> > 
> > Aka the DT is parked in some firmware part (u-boot?) and gets handed
> > to the newer kernel.
> > 
> > I don't see deliberately handing a newer DTB to an older kernel as a
> > very common occurence?
> > 
> > 
> > And from a practical standpoint:
> > 
> > We're talking about devices with DSI displays that are at stake here,
> > so we're in the embedded world.
> > 
> > In other areas we're still fighting basic functionality like actually
> > handling the type-c orientation correctly in the usbphy.
> > 
> > So I really don't see a device with a user-updateable mainline kernel out
> > in the field, apart from people hacking around on their devices to try
> > out new features and new kernels, simply because core functionality
> > is still evolving in mainline.
> > 
> > 
> > RK3576/RK3588 devices still ship with a vendor u-boot/kernel, with
> > completely incompatible DTB afterall.
> > 
> > 
> > So yes, supporting old devicetrees on new kernels I do see as important
> > but someone working extra hard to combine a new DT with an old kernel,
> > I really don't see as an issue here.
> >
> 
> Just to make sure I understood it correctly: the current suggestion
> would be the standard integer property (optional)
> 
>   phy-type:
>     description: D-PHY (default) or C-PHY mode
>     enum:
>       - 10 # PHY_TYPE_DPHY
>       - 11 # PHY_TYPE_CPHY
>     default: 10
> 
> plus changing phy-cells to:
> 
>   "#phy-cells":
>     const: 1
>     description: |
>       Argument defines whether receiver or transmitter PHY is used.
>         - PHY_DIRECTION_TX
>         - PHY_DIRECTION_RX
>       See include/dt-bindings/phy/phy.h for constants.
> 
> plus handling the case where an old DTS may still have a line
> 
>   phys = <&mipidcphy0 PHY_TYPE_DPHY>;
> 
> and the new kernel will convert it to something equal to phy-type =
> PHY_TYPE_DPHY and phys = <&mipidcphy0 PHY_DIR_TX>;
> 
> plus ignoring the whether or not an old kernel can work with a new DTS
> or not.
> 
> By the way, if we handle the case of the old DTS with the new kernel
> correctly (which we should IMHO) then we do not need to change the
> existing DTS nodes, thus eliminating regressions.
> 
> Is this summary correct?

Yep, that's what I had in mind ... though dependent on DT-maintainers
not disagreeing on the reasoning ;-)

Heiko