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

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

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.
> 

-- 
Best wishes,
Vladimir