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

[Vladimir Zapolskiy]

On 3/18/26 17:27, Dmitry Baryshkov wrote:
> 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?

As for today CAMSS CSIPHY are already media devices, and a user applies media
specific properties to them, for instance media bus format, resolution etc.
Technically this might be removed from CAMSS, but if so, then it should be
done before this new PHY driver model is applied.

--
Best wishes,
Vladimir