Re: [PATCH v13 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 21/07/2026 12:05, Vladimir Zapolskiy wrote:
> On 7/21/26 13:47, Bryan O'Donoghue wrote:
>> On 21/07/2026 10:56, Vladimir Zapolskiy wrote:
>>> On 7/21/26 12:39, Bryan O'Donoghue wrote:
>>>> On 21/07/2026 09:17, Vladimir Zapolskiy wrote:
>>>>>> +  "#phy-cells":
>>>>>> +    const: 1
>>>>>> +    description:
>>>>>> +      The single cell specifies the PHY operating mode.
>>>>> This has been reported before, #phy-cells shall be 0, the operation
>>>>> mode is selected by bus-type of endpoints as it is described in
>>>>> video-interfaces.yaml
>>>>>
>>>>> Two competing values are redundant and it opens a door to a wrong
>>>>> hardware description, the technical discussion has not been started 
>>>>> yet.
>>>> So as I said before, I'm following Rob Herring's input on this and
>>>> Krzysztof has given RB for this binding. At this stage, I don't see
>>>> scope to change it.
>>>>
>>>> Adding endpoints was already a compromise to address your feedback,
>>>> which I hoped would bring you on board with the design.
>>> That's my comment dated March 27, 2026, and it says moving phy type
>>> from phy cell to the endpoint property excludes the need to set any
>>> phy cells:
>>>
>>> https://lore.kernel.org/linux-arm-msm/e37ce438-12c7-462d-
>>> b2bc-1351be62b806@linaro.org/
>>>
>>> The technical comment is left unresponded so far since then.
>>>
>>> Since there is no expected phy cells to get on consumer's side, and
>>> since there is another link between CSIPHY and CSID described in
>>> video-interfaces.yaml, any kind of dependency of this IP on phy can
>>> be just removed, which serves the purpose of hardware description
>>> simplification.
>>
>> Rob's v1 feedback was consumer-decides-mode via the cell; Krzysztof has
>> R-b'd the binding with cells=1.
> 
> Unfortunately this is not a technical discussion to move forward
> a better hardware description.
> 
>> I'm happy to lock down this binding as-is on that basis.
>>
>> Are you arguing about phy-cells or are you actually arguing the entire
>> instantiation of Qualcomm MIPI CSI2 as a standalone generic driver in
>> drivers/phy ? Its not the same debate at all.
> 
> So far I don't review the driver, only CSIPHY IP hardware description.
> 
> In the current version of the hardware description one may find that
> the exploited 'video-intefaces' interface completely covers the 'phys'
> interface between CSIPHY and CSID, quite probably both are not needed
> at the same time, and the simpler 'phys' can be removed without any
> losses. But if it is kept, then its cell value is anyway redundant.
> 
>> As the CAMSS maintainer, I'm fully against adding more inline monolithic
>> CSIPHY init sequences. That could be represented as a separate
>> linux-media driver but, then that implies that _all_ upstream CSIPHY
>> drivers in drivers/phy are wrong, and I just don't accept that.
> 
> Other two drivers are different and should be excluded as good
> references, both of them links exactly one sensor to exactly one
> CSI-2 decoder, it's not the case here. Also one of these two drivers
> has no upstream users.
> 
>> The bindings have DT maintainer RB, and the driver has Qcom engineer
>> tested-by, review-by.
>>
>> Vinod, Kishon - Again, I'd request this merges as-is.
>>
> 

The graph and phys are different layers, endpoints describe the
media topology, phys is how CSID controls the PHY through the PHY
framework (phy_get/configure/power_on).

Removing phys doesn't simplify the description, it removes the control 
interface which is the monolith again.

We have multiple examples of this model upstream, cdns-dphy-rx + 
ti-csi2rx in-tree, with users.

I'm still kicking this up to Vinod, Kishon, request stands.

Multiple DT and driver users are RB/Tested-by on the schema and driver, 
lots of other work is gated.

It is time to merge.

Please adjudicate.

---
bod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy