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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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" ?

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

> + */
>  #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)
>  
>  /**

-- 
Regards,

Laurent Pinchart