On 17/04/2020 12:34, Kieran Bingham wrote: > From: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> > > The RDACM20 is a GMSL camera supporting 1280x800 resolution images > developed by IMI based on an Omnivision 10635 sensor and a Maxim MAX9271 > GMSL serializer. > > The GMSL link carries power, control (I2C) and video data over a > single coax cable. > > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx> > Reviewed-by: Rob Herring <robh@xxxxxxxxxx> > > --- <snip> > diff --git a/drivers/media/i2c/rdacm20.c b/drivers/media/i2c/rdacm20.c > new file mode 100644 > index 000000000000..37786998878b > --- /dev/null > +++ b/drivers/media/i2c/rdacm20.c > @@ -0,0 +1,668 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * IMI RDACM20 GMSL Camera Driver > + * > + * Copyright (C) 2017-2020 Jacopo Mondi > + * Copyright (C) 2017-2020 Kieran Bingham > + * Copyright (C) 2017-2019 Laurent Pinchart > + * Copyright (C) 2017-2019 Niklas Söderlund > + * Copyright (C) 2016 Renesas Electronics Corporation > + * Copyright (C) 2015 Cogent Embedded, Inc. > + */ > + > +/* > + * The camera is made of an Omnivision OV10635 sensor connected to a Maxim > + * MAX9271 GMSL serializer. > + */ > + > +#include <linux/delay.h> > +#include <linux/fwnode.h> > +#include <linux/init.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/slab.h> > +#include <linux/videodev2.h> > + > +#include <media/v4l2-async.h> > +#include <media/v4l2-ctrls.h> > +#include <media/v4l2-subdev.h> > + > +#include "max9271.h" > + > +#define OV10635_I2C_ADDRESS 0x30 > + > +#define OV10635_SOFTWARE_RESET 0x0103 > +#define OV10635_PID 0x300a > +#define OV10635_VER 0x300b > +#define OV10635_SC_CMMN_SCCB_ID 0x300c > +#define OV10635_SC_CMMN_SCCB_ID_SELECT BIT(0) > +#define OV10635_VERSION 0xa635 > + > +#define OV10635_WIDTH 1280 > +#define OV10635_HEIGHT 800 > +#define OV10635_FORMAT MEDIA_BUS_FMT_UYVY8_2X8 This OV10635_FORMAT define was very confusing when I reviewed this code. Please just use MEDIA_BUS_FMT_UYVY8_2X8 directly instead of introducing an alias. While reviewing I thought for a moment that OV10635_FORMAT was somehow a new mediabus format that was added elsewhere. I had to dig into the code to figure out that it really was an alias. Regards, Hans
