Re: [PATCH v2 11/11] media: ccs-pll: Document the CCS PLL flags

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Laurent,

On Mon, Apr 21, 2025 at 11:29:21PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Thu, Apr 17, 2025 at 09:53:54AM +0300, Sakari Ailus wrote:
> > Document the CCS PLL flags with short comments. The CCS spec has more
> > information on them while the added documentation helps finding the
> > relevant information in the CCS spec.
> 
> Oohhhh, documentation, that's nice :-)
> 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > ---
> >  drivers/media/i2c/ccs-pll.h | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/media/i2c/ccs-pll.h b/drivers/media/i2c/ccs-pll.h
> > index e8297db45460..6e503fe2d591 100644
> > --- a/drivers/media/i2c/ccs-pll.h
> > +++ b/drivers/media/i2c/ccs-pll.h
> > @@ -18,19 +18,40 @@
> >  #define CCS_PLL_BUS_TYPE_CSI2_DPHY				0x00
> >  #define CCS_PLL_BUS_TYPE_CSI2_CPHY				0x01
> >  
> > -/* Old SMIA and implementation specific flags */
> > -/* op pix clock is for all lanes in total normally */
> > +/* Old SMIA and implementation specific flags. */
> > +/* OP PIX clock is for all lanes in total normally. */
> >  #define CCS_PLL_FLAG_OP_PIX_CLOCK_PER_LANE			BIT(0)
> > -#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> > +
> >  /* CCS PLL flags */
> > +
> > +/* The sensor doesn't have OP clocks at all. */
> > +#define CCS_PLL_FLAG_NO_OP_CLOCKS				BIT(1)
> > +/* System speed model if this flag is unset. */
> >  #define CCS_PLL_FLAG_LANE_SPEED_MODEL				BIT(2)
> > +/* If set, the pre-PLL divider may have odd values, too. */
> >  #define CCS_PLL_FLAG_EXT_IP_PLL_DIVIDER				BIT(4)
> > +/*
> > + * If set, the OP PIX clock doesn't have to exactly match with data rate, it may
> > + * be higher. See "OP Domain Formulas" in MIPI CCS 1.1 spec.
> > + */
> >  #define CCS_PLL_FLAG_FLEXIBLE_OP_PIX_CLK_DIV			BIT(5)
> > +/* If set, the VT domain may run faster than the OP domain. */
> >  #define CCS_PLL_FLAG_FIFO_DERATING				BIT(6)
> > +/* If set, the VT domain may run slower than the OP domain. */
> >  #define CCS_PLL_FLAG_FIFO_OVERRATING				BIT(7)
> > +/* If set, the PLL tree has two PLLs instead of one. */
> >  #define CCS_PLL_FLAG_DUAL_PLL					BIT(8)
> > +/*
> > + * If set, the OP SYS clock is a dual data rate clock, transferring two bits per
> > + * cycle instead of one.
> > + */
> >  #define CCS_PLL_FLAG_OP_SYS_DDR					BIT(9)
> > +/*
> > + * If set, the OP PIX clock is a dual data rate clock, transferring two bits per
> > + * cycle instead of one.
> 
> Should this be "two pixels per cycle" ?

Good catch. I'll fix that for v3.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thank you!

> 
> > + */
> >  #define CCS_PLL_FLAG_OP_PIX_DDR					BIT(10)
> > +/* If set, the PLL multipliers are required to be even. */
> >  #define CCS_PLL_FLAG_EVEN_PLL_MULTIPLIER			BIT(11)
> >  
> >  /**
> 

-- 
Kind regards,

Sakari Ailus