Re: [PATCH 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI D-PHY driver

[neil.armstrong@linaro.org]

On 15/07/2025 11:33, Konrad Dybcio wrote:
> On 7/15/25 11:20 AM, Vladimir Zapolskiy wrote:
>> On 7/15/25 12:01, Konrad Dybcio wrote:
>>> On 7/15/25 8:35 AM, Vladimir Zapolskiy wrote:
>>>> On 7/15/25 03:13, Bryan O'Donoghue wrote:
>>>>> On 14/07/2025 16:30, Vladimir Zapolskiy wrote:
>>>>>>>
>>>>>>> I think that is genuinely something we should handle in camss-csid.c
>>>>>>> maybe with some meta-data inside of the ports/endpoints..
>>>>>>>
>>>>>>
>>>>>> This is a CSIPHY property, a CSIPHY hardware configuration and a wiring
>>>>>> of sensors to a CSIPHY. Where is the relation to CSID here? There is no.
>>>>>
>>>>> All the PHY really needs to know is the # of lanes in aggregate, which
>>>>> physical lanes to map to which logical lanes and the pixel clock.
>>>>>
>>>>> We should add additional support to the Kernel's D-PHY API parameters
>>>>> mechanism to support that physical-to-logical mapping but, that's not
>>>>> required for this series or for any currently know upstream user of CAMSS.
>>>>>
>>>>>> Please share at least a device tree node description, which supports
>>>>>> a connection of two sensors to a single CSIPHY, like it shall be done
>>>>>> expectedly.
>>>>> &camss {
>>>>>         port@0 {
>>>>>             csiphy0_lanes01_ep: endpoint0 {
>>>>>                 data-lanes = <0 1>;
>>>>>                 remote-endpoint = <&sensor0_ep>;
>>>>>             };
>>>>>
>>>>>             csiphy0_lanes23_ep: endpoint0 {
>>>>>                 data-lanes = <2 3>;
>>>>>                 remote-endpoint = <&sensor1_ep>;
>>>>>             };
>>>>>          };
>>>>> };
>>>>
>>>> Don't you understand that this is broken?.. That's no good.
>>>>
>>>> Please listen and reread the messages given to you above, your proposed
>>>> "solution" does not support by design a valid hardware setup of two
>>>> sensors connected to the same CSIPHY.
>>>>
>>>> I would propose to stop force pushing an uncorrectable dt scheme, it
>>>> makes no sense.
>>>
>>> If all you're asking for is an ability to grab an of_graph reference
>>> from the camss (v4l2) driver, you can simply do something along the
>>> lines of of_graph_get_remote_port(phy->dev->of_node)
>>>
>>
>> It's not about the driver specifics, my comment is about a proper
>> hardware description in dts notation, please see the device tree node
>> names.
> 
> I'm a little lost on what you're trying to argue for..
> 
> I could make out:
> 
> 1. "the phy should be a multimedia device"
> 2. "There is no ports at all, which makes the device tree node unusable,
>    since you can not provide a way to connect any sensors to the phy."
> 
> I don't really understand #1.. maybe that could be the case if the PHY
> has a multitude of tunables (which I don't know if it does, but wouldn't
> be exactly surprised if it did) that may be usecase/pipeline-specific
> 
> As for #2, I do think it makes sense to connect the sensors to the PHY,
> as that's a representation of electrical signals travelling from the
> producer to the consumer (plus the data passed in e.g. data-lanes is
> directly related to the PHY and necessarily consumed by its driver)

The port/endpoint should represent the data flow, and if the signal is the following:

sensor -> csiphy -> csid

and in addition the "csiphy" can handle up to two sensors by splitting the lanes,
then the DT should take this in account and effectively the "csiphy" should be
an element of the graph and should take it's configuration from the DT graph
to properly configure the lanes configuration.

While it can feel simpler to simply move the csiphy code into standalone
PHY device and just replace camss code with phy_*() calls, it doesn't
solve the proper representation on the data flow and doesn't fix how
the combo mode could be handled.

The solution shared by Vladimir (I think the "csiphy" node should be out
of the camss node), solves all this and will be able to handle more
complex situations with shared resources between "csiphys" and start
moving elements out of the gigantic camss node.

Neil


> 
> Konrad
>