[PATCH v4 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
Bryan O'Donoghue
bod at kernel.org
Wed Mar 18 08:47:53 PDT 2026
On 18/03/2026 15:07, Neil Armstrong wrote:
> On 3/18/26 14:17, Bryan O'Donoghue wrote:
>> On 18/03/2026 10:15, Neil Armstrong wrote:
>>>> + /*
>>>> + * phy_configure_opts_mipi_dphy.lanes starts from zero to
>>>> + * the maximum number of enabled lanes.
>>>> + *
>>>> + * TODO: add support for bitmask of enabled lanes and polarities
>>>> + * of those lanes to the phy_configure_opts_mipi_dphy struct.
>>>> + * For now take the polarities as zero and the position as fixed
>>>> + * this is fine as no current upstream implementation maps otherwise.
>>>> + */
>>>
>>> This is wrong since you loose the lanes mapping defined in DT, which is still in CAMSS
>>> but is a PHY property. The lanes layout is not a property of the CSI controller,
>>> CSI controller only need to know the lanes count, and not the layout.
>>
>> Lane layout is a PHY concern but, the PHY API gives us phy_configure_opts_mipi_dphy which should be extended to provide layout and polarity. This would then be of benefit to more than just qcom/camss.
>
> Why ? the only concern between a controller and a PHY is the lane count to calculate the bandwidth, the actual pin layout is certainly not a controller concern.
Controllers already get the lane count by way of data-lanes = <x y z q>
or <x y> or <x> if we didn't do that we would need to specify the
data-lanes in the controller and again in the PHY.
>>
>> Right now none of the CAMSS users for this driver depend on any other mapping and I propose a separate series to fix phy_configure_opts_mipi_dphy rather than introduce data-lanes to DPHY.
>
> None of the upstream users of camss.
No, we are establishing from x1 use of standard drivers/phy. New users
will do it this way. The posted dtsi for the laptops can use the linear
lane layout and default polarities.
In a follow on series we can extend phy_configure_opts_mipi_dphy to
parse data-lanes = <> into count and mask, to the benefit of any user of
phy_configure_opts_mipi_dphy.
Since that will touch more then qcom specific stuff and will touch at
least two subsystems, that should be its own separate series.
> The problem is even larger, as you replied in [1], the csiphy is still exposed as a media element from the CAMSS driver, this means this driver is not complete,
> it should be a media driver entirely with eventually an internal PHY aux driver, but this would be entirely implementation specific.
>
> Either the PHY is standalone and the PHY consumer only calls phy_open/init/configure/power_on/power_off/exit, otherwise it's not a fully standaline PHY but a composite device like here.
This is not a composite device any more than the existing upstream
implementations which follow the same model:
- Cadence CSI2RX + Cadence DPHY (TI J721E/AM62A)
- Rockchip rkisp1 + phy-rockchip-inno-csidphy
Both use phys = <&phandle>, the media driver manages V4L2 endpoints
and lane counts, the PHY driver handles the electrical layer via
phy_configure().
To this list we will add qcom camss, there's nothing exotic being proposed.
> I propose that you write a proper media driver for the qcom csiphy, which eventually spins a PHY driver as an aux device.
None of these SoC D-PHYs are written as V4L2 media drivers that spawn
auxiliary devices. They all use the phys = <&phandle> model. The media
driver manages the V4L2 endpoints and lane counts, passing the
configuration down via phy_configure_opts_mipi_dphy.
I just don't see what is so special about CAMSS that it needs to have
its own special PHY implementation. drivers/phy the standard API and
specification of data-lanes etc in the controller seems pretty "bog
standard".
---
bod
More information about the linux-phy
mailing list