Hi Andrzej, Please see some minor comments inline. On Wednesday 21 of August 2013 16:41:31 Andrzej Hajda wrote: > Driver for Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor > with embedded SoC ISP. > The driver exposes the sensor as two V4L2 subdevices: > - S5K5BAF-CIS - pure CMOS Image Sensor, fixed 1600x1200 format, > no controls. > - S5K5BAF-ISP - Image Signal Processor, formats up to 1600x1200, > pre/post ISP cropping, downscaling via selection API, controls. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> > --- > Hi, > > This patch incorporates Stephen's suggestions, thanks. > > Regards > Andrzej > > v7 > - changed description of 'clock-frequency' DT property > > v6 > - endpoint node presence is now optional, > - added asynchronous subdev registration support and clock > handling, > - use named gpios in DT bindings > > v5 > - removed hflip/vflip device tree properties > > v4 > - GPL changed to GPLv2, > - bitfields replaced by u8, > - cosmetic changes, > - corrected s_stream flow, > - gpio pins are no longer exported, > - added I2C addresses to subdev names, > - CIS subdev registration postponed after > succesfull HW initialization, > - added enums for pads, > - selections are initialized only during probe, > - default resolution changed to 1600x1200, > - state->error pattern removed from few other functions, > - entity link creation moved to registered callback. > > v3: > - narrowed state->error usage to i2c and power errors, > - private gain controls replaced by red/blue balance user controls, > - added checks to devicetree gpio node parsing > > v2: > - lower-cased driver name, > - removed underscore from regulator names, > - removed platform data code, > - v4l controls grouped in anonymous structs, > - added s5k5baf_clear_error function, > - private controls definitions moved to uapi header file, > - added v4l2-controls.h reservation for private controls, > - corrected subdev registered/unregistered code, > - .log_status sudbev op set to v4l2 helper, > - moved entity link creation to probe routines, > - added cleanup on error to probe function. > --- > .../devicetree/bindings/media/samsung-s5k5baf.txt | 59 + > MAINTAINERS | 7 + > drivers/media/i2c/Kconfig | 7 + > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/s5k5baf.c | 2045 > ++++++++++++++++++++ 5 files changed, 2119 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/samsung-s5k5baf.txt create mode > 100644 drivers/media/i2c/s5k5baf.c > > diff --git a/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt > b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt new file > mode 100644 > index 0000000..d680d99 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/samsung-s5k5baf.txt > @@ -0,0 +1,59 @@ > +Samsung S5K5BAF UXGA 1/5" 2M CMOS Image Sensor with embedded SoC ISP > +-------------------------------------------------------------------- > + > +Required properties: > + > +- compatible : "samsung,s5k5baf"; > +- reg : I2C slave address of the sensor; Can this sensor have an aribitrary slave address or only a set of well known possible addresses (e.g. listed in documentation)? > +- vdda-supply : analog power supply 2.8V (2.6V to 3.0V); > +- vddreg-supply : regulator input power supply 1.8V (1.7V to 1.9V) > + or 2.8V (2.6V to 3.0); > +- vddio-supply : I/O power supply 1.8V (1.65V to 1.95V) > + or 2.8V (2.5V to 3.1V); > +- stbyn-gpios : GPIO connected to STDBYN pin; > +- rstn-gpios : GPIO connected to RSTN pin; Both GPIOs above have names suggesting that they are active low. I wonder how the GPIO flags cell is interpreted here, namely the polarity bit. Otherwise the binding looks good. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
