Hi!

From: Sakari Ailus <sakari.ailus@iki.fi>

That address no longer works, right?

Add a LED flash class driver for the as3654a flash controller. A V4L2 flash
driver for it already exists (drivers/media/i2c/as3645a.c), and this driver
is based on that.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

+ * Based on drivers/media/i2c/as3645a.c.
+ *
+ * Contact: Sakari Ailus [off-list ref]

So I believe it should not be here.

+/*
+ * as3645a_set_config - Set flash configuration registers
+ * @flash: The flash
+ *

/** for linuxdoc? 

+	struct as3645a *flash = fled_to_as3645a(fled);
+	int rval;
+
+	/* NOTE: reading register clear fault status */

clears.

+static unsigned int as3645a_current_to_reg(struct as3645a *flash, bool is_flash,
+					   unsigned int ua)
+{
+	struct {
+		unsigned int min;
+		unsigned int max;
+		unsigned int step;
+	} __mms[] = {
+		{
+			AS_TORCH_INTENSITY_MIN,
+			flash->cfg.assist_max_ua,
+			AS_TORCH_INTENSITY_STEP
+		},
+		{
+			AS_FLASH_INTENSITY_MIN,
+			flash->cfg.flash_max_ua,
+			AS_FLASH_INTENSITY_STEP
+		},
+	}, *mms = &__mms[is_flash];
+
+	if (ua < mms->min)
+		ua = mms->min;

That's some... seriously interesting code. And you are forcing gcc to
create quite interesting structure on stack. Would it be easier to do
normal if()... without this magic?

+	struct v4l2_flash_config cfg = {
+		.torch_intensity = {
+			.min = AS_TORCH_INTENSITY_MIN,
+			.max = flash->cfg.assist_max_ua,
+			.step = AS_TORCH_INTENSITY_STEP,
+			.val = flash->cfg.assist_max_ua,
+		},
+		.indicator_intensity = {
+			.min = AS_INDICATOR_INTENSITY_MIN,
+			.max = flash->cfg.indicator_max_ua,
+			.step = AS_INDICATOR_INTENSITY_STEP,
+			.val = flash->cfg.indicator_max_ua,
+		},
+	};

Ugh. And here you have copy of the above struct, + .val. Can it be
somehow de-duplicated?

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html