Re: [PATCH v2 10/11] arm64: dts: qcom: x1e80100-lenovo-yoga-slim7x: Add ov02c10 RGB sensor on CSIPHY4

[Konrad Dybcio]

On 3/18/26 10:26 AM, Neil Armstrong wrote:
> On 3/16/26 12:46, Bryan O'Donoghue wrote:
>> On 16/03/2026 08:28, Neil Armstrong wrote:
>>>> +    ports {
>>>> +        /*
>>>> +         * port0 => csiphy0
>>>> +         * port1 => csiphy1
>>>> +         * port2 => csiphy2
>>>> +         * port3 => csiphy4
>>>> +         */
>>
>> Hi.
>>
>> Thanks for the review.
>>
>> I think the above comment probably isn't making this very clear.
>>
>> port0 => csiphy0 => msm_csiphy0 in the media-graph.
>>
>>>> +        port@3 {
>>>> +            camss_csiphy4_inep0: endpoint@0 {
>>>> +                clock-lanes = <7>;
>>>> +                data-lanes = <0 1>;
>>>> +                remote-endpoint = <&ov02c10_ep>;
>>>
>>> This is quite wrong, with the PHY in a separate node, the lanes layout has nothing
>>> to do in the "controller" ports since the sensor is connected to the the PHY which
>>> configures the lanes functions.
>>>
>>> The PHY should be a media element in a port/endpoint chain to properly describe the
>>> data flow from the sensor to the controller.
>>
>> If I am reading your comment right, we are already defining the data-lanes where you've said they should be msm_csiphyX below port@X here maps to msm_csiphyX in the media graph.
>>
>> So for example here is how we configure this before and after the changes in this series
>>
>> media-ctl -v -d /dev/media0 -V '"ov08x40 '2-0036'":0[fmt:SGRBG10/3856x2416 field:none]'
>> media-ctl -V '"msm_csiphy4":0[fmt:SGRBG10/3856x2416]'
>> media-ctl -V '"msm_csid0":0[fmt:SGRBG10/3856x2416]'
>> media-ctl -V '"msm_vfe0_rdi0":0[fmt:SGRBG10/3856x2416]'
>> media-ctl -l '"msm_csiphy4":1->"msm_csid0":0[1]'
>> media-ctl -l '"msm_csid0":1->"msm_vfe0_rdi0":0[1]'
>> media-ctl -d /dev/media0 -p
> 
> So a csiphy is a media element here, so why implement it as a PHY ? and there's a data link with CSID with should represented with a port/endpoint relationship...

Does it need to be a media element in the first place?

If it's just about configuration, then we can (?) make an assumption
that the PHY is hardwired to the sensor, and if we want/need to mux
CSIDn to the sensor device, which would then trigger a change in the
PHY (if the PHY even needs to know it's being remuxed?)

i.e.

the above sequence would have:

media-ctl -l '"ov08x40 '2-0036'":1->"msm_csid0":0[1]'

Konrad