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

[Dmitry Baryshkov]

On Wed, Mar 18, 2026 at 04:07:39PM +0100, 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.

I think that the DT should be providing the information about the
connection of the lanes and their number on the board. Then the CSI host
might want to limit this further for whatever reasons. But I don't think
that the properties of the lanes should be configurable between the
controller and 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.
> 
> 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.
> 
> I propose that you write a proper media driver for the qcom csiphy, which eventually spins a PHY driver as an aux device.

Why do you want a media driver? Isn't PHY driver enough?

-- 
With best wishes
Dmitry