Hi Laurent Quoting Laurent Pinchart (2022-01-23 16:08:52) > There are many CSI-2-related drivers in the media subsystem that come > with their own macros to handle the CSI-2 data types (or just hardcode > the numerical values). Provide a shared header with definitions for > those data types that driver can use. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > --- > include/media/mipi-csi2.h | 45 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 45 insertions(+) > create mode 100644 include/media/mipi-csi2.h > > diff --git a/include/media/mipi-csi2.h b/include/media/mipi-csi2.h > new file mode 100644 > index 000000000000..392794e5badd > --- /dev/null > +++ b/include/media/mipi-csi2.h > @@ -0,0 +1,45 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * MIPI CSI-2 Data Types > + * > + * Copyright (C) 2022 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > + */ > + > +#ifndef _MEDIA_MIPI_CSI2_H > +#define _MEDIA_MIPI_CSI2_H > + > +/* Short packet data types */ > +#define MIPI_CSI2_DT_FS 0x00 > +#define MIPI_CSI2_DT_FE 0x01 > +#define MIPI_CSI2_DT_LS 0x02 > +#define MIPI_CSI2_DT_LE 0x03 > +#define MIPI_CSI2_DT_GENERIC_SHORT(n) (0x08 + (n)) /* 0..7 */ > + > +/* Long packet data types */ > +#define MIPI_CSI2_DT_NULL 0x10 > +#define MIPI_CSI2_DT_BLANKING 0x11 > +#define MIPI_CSI2_DT_EMBEDDED_8B 0x12 > +#define MIPI_CSI2_DT_YUV420_8B 0x18 > +#define MIPI_CSI2_DT_YUV420_10B 0x19 > +#define MIPI_CSI2_DT_YUV420_8B_LEGACY 0x1a > +#define MIPI_CSI2_DT_YUV420_8B_CS 0x1c > +#define MIPI_CSI2_DT_YUV420_10B_CS 0x1d > +#define MIPI_CSI2_DT_YUV422_8B 0x1e > +#define MIPI_CSI2_DT_YUV422_10B 0x1f > +#define MIPI_CSI2_DT_RGB444 0x20 > +#define MIPI_CSI2_DT_RGB555 0x21 > +#define MIPI_CSI2_DT_RGB565 0x22 > +#define MIPI_CSI2_DT_RGB666 0x23 > +#define MIPI_CSI2_DT_RGB888 0x24 > +#define MIPI_CSI2_DT_RAW24 0x27 > +#define MIPI_CSI2_DT_RAW6 0x28 > +#define MIPI_CSI2_DT_RAW7 0x29 > +#define MIPI_CSI2_DT_RAW8 0x2a > +#define MIPI_CSI2_DT_RAW10 0x2b > +#define MIPI_CSI2_DT_RAW12 0x2c > +#define MIPI_CSI2_DT_RAW14 0x2d > +#define MIPI_CSI2_DT_RAW16 0x2e > +#define MIPI_CSI2_DT_RAW20 0x2f > +#define MIPI_CSI2_DT_USER_DEFINED(n) (0x30 + (n)) /* 0..7 */ I don't have an easy way to validate those values right now so as with Niklas I'll leave those to your judgement, and Pratyush's review. Also along side Pratyush's comment, I concur that the mapping tables too could be common, but I suspect that's an even bigger topic as maybe that falls into the trap of also being common to DRM formats... And finally, are these defines in a location that can be accessible from device tree? Or would it have to be further duplicated there still? For instance, the bindings for the Xilinx CSI2 RX explicitly list DT values to specify as the xlnx,csi-pxl-format which I think should also come from this common header definition. For the patches here so far, I can't see anything stark that is wrong so for the series: Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> as further extending this to the device tree bindings can be done on top. > + > +#endif /* _MEDIA_MIPI_CSI2_H */ > -- > Regards, > > Laurent Pinchart >
