On Mon, Nov 13, 2023 at 03:53:22PM +0200, Laurent Pinchart wrote:
> On Mon, Nov 13, 2023 at 03:44:54PM +0200, Sakari Ailus wrote:
> > Provide a few bits for drivers to store private information on register
> > definitions.
> >
> > Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> > Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > ---
> > include/media/v4l2-cci.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/include/media/v4l2-cci.h b/include/media/v4l2-cci.h
> > index f2c2962e936b..ee469f03e440 100644
> > --- a/include/media/v4l2-cci.h
> > +++ b/include/media/v4l2-cci.h
> > @@ -33,6 +33,11 @@ struct cci_reg_sequence {
> > #define CCI_REG_ADDR_MASK GENMASK(15, 0)
> > #define CCI_REG_WIDTH_SHIFT 16
> > #define CCI_REG_WIDTH_MASK GENMASK(19, 16)
>
> Should these be unsigned too for consistency ?
>
> > +/*
> > + * Private CCI register flags, for the use of drivers.
> > + */
> > +#define CCI_REG_PRIVATE_SHIFT 28U
> > +#define CCI_REG_PRIVATE_MASK GENMASK(31U, CCI_REG_PRIVATE_SHIFT)
And actually, for consistency again, I would write
#define CCI_REG_PRIVATE_MASK GENMASK(31U, 28U)
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
> > #define CCI_REG8(x) ((1 << CCI_REG_WIDTH_SHIFT) | (x))
> > #define CCI_REG16(x) ((2 << CCI_REG_WIDTH_SHIFT) | (x))
--
Regards,
Laurent Pinchart
