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

[Dmitry Baryshkov]

On Fri, Mar 27, 2026 at 11:07:53AM +0100, Konrad Dybcio wrote:
> On 3/27/26 3:23 AM, Hangxiang Ma wrote:
> > On 3/26/2026 9:04 AM, Bryan O'Donoghue wrote:
> >> +#include <linux/delay.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/io.h>
> >> +#include <linux/time64.h>
> >> +
> >> +#include "phy-qcom-mipi-csi2.h"
> >> +
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n)    ((offset) + 0x4 * (n))
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET    BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE    BIT(7)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B    BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID    BIT(1)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD    BIT(0)
> >> +#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n)    ((offset) + 0xb0 + 0x4 * (n))
> >>
> > Hi Bryan, one minor observation on the following macro:
> > 
> >     CSIPHY_3PH_CMN_CSI_COMMON_STATUSn
> > 
> > The 0xb0 offset implicitly assumes a fixed distance between the
> > common_ctrl and common_status register blocks. This holds for the PHYs
> > covered by this series, but on some other platforms (e.g. Kaanapali,
> > Pakala) the offset differs.
> > 
> > That said, I think keeping this fixed value is reasonable for the scope
> > of the current PHY series, and it does help keep the macro set simple.
> > It might just be worth documenting this assumption (e.g. via a comment
> > or in the commit message).
> > 
> > Alternatively, if future PHY variants need to support different layouts,
> > this could be made more extensible by moving the status base offset into
> > the per-PHY data (similar to other register layout parameters). But I
> > don’t think that needs to block the current series.
> 
> If the register contents are generally similar but the bit positions
> and/or reg offsets differ, regmap_fields may be useful

Or platform-specific set of macros / reg accessors (as it was done in
QMP).

> 
> Konrad

-- 
With best wishes
Dmitry