Thanks for the comments. A quick question first. For the reset we need some time to address. -----Original Message----- From: jacopo mondi [mailto:jacopo@xxxxxxxxxx] Sent: Tuesday, March 20, 2018 6:28 PM To: Yeh, Andy <andy.yeh@xxxxxxxxx> Cc: linux-media@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Chiang, AlanX <alanx.chiang@xxxxxxxxx> Subject: Re: RESEND[PATCH v6 2/2] media: dw9807: Add dw9807 vcm driver Hi Andy, a few comments on you patch below... On Sat, Mar 17, 2018 at 01:05:26AM +0800, Andy Yeh wrote: > From: Alan Chiang <alanx.chiang@xxxxxxxxx> > a/drivers/media/i2c/dw9807.c b/drivers/media/i2c/dw9807.c new file > mode 100644 index 0000000..95626e9 > --- /dev/null > +++ b/drivers/media/i2c/dw9807.c > @@ -0,0 +1,320 @@ > +// Copyright (C) 2018 Intel Corporation // SPDX-License-Identifier: > +GPL-2.0 > + Nit: my understanding is that the SPDX identifier goes first https://lwn.net/Articles/739183/ I checked this site. And it says Copyright should be before SPDX identifier. ========== file01.c ========== // Copyright (c) 2012-2016 Joe Random Hacker // SPDX-License-Identifier: BSD-2-Clause ... ========== file02.c ========== // Copyright (c) 2017 Jon Severinsson // SPDX-License-Identifier: BSD-2-Clause ... ========== file03.c ========== // Copyright (c) 2008 The NetBSD Foundation, Inc. // SPDX-License-Identifier: BSD-2-Clause-NetBSD > +#include <linux/acpi.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > +#include <media/v4l2-ctrls.h> > +#include <media/v4l2-device.h> > + > +#define DW9807_NAME "dw9807" > +#define DW9807_MAX_FOCUS_POS 1023 >