Re: [PATCH RFC] media: imx355: reuse existing CCS defines

[David Heidelberg]

On 26/08/2026 08:20, Jai Luthra wrote:
> Hi David,
>
> Quoting David Heidelberg via B4 Relay (2026-08-20 01:47:08)
> > From: David Heidelberg <david@xxxxxxx>
> >
> > The driver may not be MIPU CCS compliant, but does use same address and
> > often set same values as compliant drivers. Do not define for every Sony
> > imx* driver registers we already know and are standard.
> >
> > Signed-off-by: David Heidelberg <david@xxxxxxx>
> > ---
> > I'm sending this as RFC, because we'll need to upstream at least 4 - 6
> > drivers, which aren't compliant with MIPI CCS. Quirking these drivers in
> > mipi-ccs would be pointless, thou we could at least simplify and unify
> > what needs to be separate.
>
> Indeed, while working on IMX708 I realized it was in the same bucket, and
> ended up implementing a similar approach (of reusing register defines and
> helpers from CCS).
>
> > Thanks for the feedback, just did few lines for the idea how the final
> > changes will look like. Anything ambiguous will be left as is.
> > ---
> >   drivers/media/i2c/imx355.c | 46 +++++++++++++++-------------------------------
> >   1 file changed, 15 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
> > index 8eb8588cb71bb..57f7c70d18453 100644
> > --- a/drivers/media/i2c/imx355.c
> > +++ b/drivers/media/i2c/imx355.c
> > @@ -14,50 +14,34 @@
> >   #include <linux/unaligned.h>
> >   
> >   #include <media/v4l2-cci.h>
> >   #include <media/v4l2-ctrls.h>
> >   #include <media/v4l2-device.h>
> >   #include <media/v4l2-event.h>
> >   #include <media/v4l2-fwnode.h>
> >   
> > -#define IMX355_REG_MODE_SELECT         CCI_REG8(0x0100)
> > -#define IMX355_MODE_STANDBY            0x00
> > -#define IMX355_MODE_STREAMING          0x01
> > +#include "ccs/ccs-regs.h"
> >   
> > -/* Chip ID */
> > -#define IMX355_REG_CHIP_ID             CCI_REG16(0x0016)
> >   #define IMX355_CHIP_ID                 0x0355
> >   
> > -#define IMX355_REG_LANE_SEL            CCI_REG8(0x0114)
> > -
> >   /* PLL registers that depend on the external clock frequency */
> >   #define IMX355_REG_EXTCLK_FREQ         CCI_REG16(0x0136)
> >   #define IMX355_REG_PLL_OP_PREDIV       CCI_REG8(0x030d)
> > -#define IMX355_REG_PLL_OP_MUL          CCI_REG16(0x030e)
> >   #define IMX355_REG_PLL_IVT_PCK_DIV     CCI_REG8(0x0301)
> >   #define IMX355_REG_PLL_IVT_SYSCK_DIV   CCI_REG8(0x0303)
>
> I think rest of the PLL registers could also be reused here?
>
> While they are defined as CCI_REG16, I think writing just a byte to it
> should work okay.

It could work, but I wouldn't risk some undefined behavior. The datasheet define 
it as is, so I would go against it (at least without seeing bigger benefit).

> You could also reuse the CCS PLL calculation helpers instead of hardcoding
> the values.

I tried, but I would skip it for now, so we keep the verified configurations 
against datasheet. Maybe in the future if there is need for it and we can 
validate against more configurations.

I'm sending v2 cleaning the driver up with the basic scope :)

Thank you
David

> [snip]
>
> Thanks,
>      Jai