From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pali =?utf-8?B?Um9ow6Fy?= Subject: Re: [PATCHv3] support for AD5820 camera auto-focus coil Date: Mon, 23 May 2016 09:41:32 +0200 Message-ID: <20160523074132.GD29844@pali> References: <20160517181927.GA28741@amd> <20160521054336.GA27123@amd> <573FFF51.1000004@gmail.com> <20160521105607.GA20071@amd> <574049EF.2090208@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <574049EF.2090208@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Pavel Machek Cc: Ivaylo Dimitrov , linux-media@vger.kernel.org, khilman@kernel.org, tony@atomide.com, mchehab@osg.samsung.com, aaro.koskinen@iki.fi, sre@kernel.org, kernel list , sakari.ailus@iki.fi, linux-omap@vger.kernel.org, patrikbachan@gmail.com, linux-arm-kernel , serge@hallyn.com List-Id: linux-omap@vger.kernel.org T24gU2F0dXJkYXkgMjEgTWF5IDIwMTYgMTQ6NDM6NDMgSXZheWxvIERpbWl0cm92IHdyb3RlOgo+ ID5kaWZmIC0tZ2l0IGEvaW5jbHVkZS9tZWRpYS9hZDU4MjAuaCBiL2luY2x1ZGUvbWVkaWEvYWQ1 ODIwLmgKPiA+bmV3IGZpbGUgbW9kZSAxMDA2NDQKPiA+aW5kZXggMDAwMDAwMC4uZjVhMTU2NQo+ ID4tLS0gL2Rldi9udWxsCj4gPisrKyBiL2luY2x1ZGUvbWVkaWEvYWQ1ODIwLmgKPiA+QEAgLTAs MCArMSw3MCBAQAo+ID4rLyoKPiA+KyAqIGluY2x1ZGUvbWVkaWEvYWQ1ODIwLmgKPiA+KyAqCj4g PisgKiBDb3B5cmlnaHQgKEMpIDIwMDggTm9raWEgQ29ycG9yYXRpb24KPiA+KyAqIENvcHlyaWdo dCAoQykgMjAwNyBUZXhhcyBJbnN0cnVtZW50cwo+ID4rICoKPiA+KyAqIENvbnRhY3Q6IFR1dWtr YSBUb2l2b25lbiA8dHV1a2thLm8udG9pdm9uZW5Abm9raWEuY29tPgo+ID4rICogICAgICAgICAg U2FrYXJpIEFpbHVzIDxzYWthcmkuYWlsdXNAbm9raWEuY29tPgo+ID4rICoKPiA+KyAqIEJhc2Vk IG9uIGFmX2Q4OC5jIGJ5IFRleGFzIEluc3RydW1lbnRzLgo+ID4rICoKPiA+KyAqIFRoaXMgcHJv Z3JhbSBpcyBmcmVlIHNvZnR3YXJlOyB5b3UgY2FuIHJlZGlzdHJpYnV0ZSBpdCBhbmQvb3IKPiA+ KyAqIG1vZGlmeSBpdCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBHZW5lcmFsIFB1YmxpYyBM aWNlbnNlCj4gPisgKiB2ZXJzaW9uIDIgYXMgcHVibGlzaGVkIGJ5IHRoZSBGcmVlIFNvZnR3YXJl IEZvdW5kYXRpb24uCj4gPisgKgo+ID4rICogVGhpcyBwcm9ncmFtIGlzIGRpc3RyaWJ1dGVkIGlu IHRoZSBob3BlIHRoYXQgaXQgd2lsbCBiZSB1c2VmdWwsIGJ1dAo+ID4rICogV0lUSE9VVCBBTlkg V0FSUkFOVFk7IHdpdGhvdXQgZXZlbiB0aGUgaW1wbGllZCB3YXJyYW50eSBvZgo+ID4rICogTUVS Q0hBTlRBQklMSVRZIG9yIEZJVE5FU1MgRk9SIEEgUEFSVElDVUxBUiBQVVJQT1NFLiAgU2VlIHRo ZSBHTlUKPiA+KyAqIEdlbmVyYWwgUHVibGljIExpY2Vuc2UgZm9yIG1vcmUgZGV0YWlscy4KPiA+ KyAqCj4gPisgKiBZb3Ugc2hvdWxkIGhhdmUgcmVjZWl2ZWQgYSBjb3B5IG9mIHRoZSBHTlUgR2Vu ZXJhbCBQdWJsaWMgTGljZW5zZQo+ID4rICogYWxvbmcgd2l0aCB0aGlzIHByb2dyYW07IGlmIG5v dCwgd3JpdGUgdG8gdGhlIEZyZWUgU29mdHdhcmUKPiA+KyAqIEZvdW5kYXRpb24sIEluYy4sIDUx IEZyYW5rbGluIFN0LCBGaWZ0aCBGbG9vciwgQm9zdG9uLCBNQQo+ID4rICogMDIxMTAtMTMwMSBV U0EKPiA+KyAqLwo+ID4rCj4gPisjaWZuZGVmIEFENTgyMF9ICj4gPisjZGVmaW5lIEFENTgyMF9I Cj4gPisKPiA+KyNpbmNsdWRlIDxsaW51eC9pMmMuaD4KPiA+KyNpbmNsdWRlIDxsaW51eC9tdXRl eC5oPgo+ID4rI2luY2x1ZGUgPGxpbnV4L3ZpZGVvZGV2Mi5oPgo+ID4rCj4gPisjaW5jbHVkZSA8 bWVkaWEvdjRsMi1jdHJscy5oPgo+ID4rI2luY2x1ZGUgPG1lZGlhL3Y0bDItc3ViZGV2Lmg+Cj4g PisKPiA+K3N0cnVjdCByZWd1bGF0b3I7Cj4gPisKPiA+KyNkZWZpbmUgQUQ1ODIwX05BTUUJCSJh ZDU4MjAiCj4gPisjZGVmaW5lIEFENTgyMF9JMkNfQUREUgkJKDB4MTggPj4gMSkKCk1heWJlIHdy aXRlIEkyQyBhZGRyZXNzIGlzIG1vcmUgcmVhZGFibGUgZm9ybT8gV2hhdCBpcyByZWFzb24gc3Vj aApiaXQgc2hpZnQgZm9ybWF0PwoKPiA+Ky8qIFJlZ2lzdGVyIGRlZmluaXRpb25zICovCj4gPisj ZGVmaW5lIEFENTgyMF9QT1dFUl9ET1dOCQkoMSA8PCAxNSkKPiA+KyNkZWZpbmUgQUQ1ODIwX0RB Q19TSElGVAkJNAo+IAo+IERvIHRob3NlIGRlZmluZXMgcmVhbGx5IGJlbG9uZyBoZXJlPyBJc24n dCBpdCBiZXR0ZXIgaWYgdGhleSBhcmUgbW92ZWQgdG8KPiBhZDU4MjAuYz8KCkZvciBtZSBpdCBs b29rcyBsaWtlIHRoaXMgaXMgcHJpdmF0ZSBmb3IgYWQ1ODIwLmMuCgo+ID4rI2RlZmluZSBBRDU4 MjBfUkFNUF9NT0RFX0xJTkVBUgkJKDAgPDwgMykKPiA+KyNkZWZpbmUgQUQ1ODIwX1JBTVBfTU9E RV82NF8xNgkJKDEgPDwgMykKPiA+Kwo+ID4rc3RydWN0IGFkNTgyMF9wbGF0Zm9ybV9kYXRhIHsK PiA+KwlpbnQgKCpzZXRfeHNodXRkb3duKShzdHJ1Y3QgdjRsMl9zdWJkZXYgKnN1YmRldiwgaW50 IHNldCk7Cj4gPit9OwoKVGhpcyBpcyBmb3IgbGVnYWN5IGJvYXJkIGNvZGUgc3VwcG9ydCByaWdo dD8gV2UgbmVlZCBEVCBzdXBwb3J0IGZvciBOOTAwCmFzIGxlZ2FjeSBib2FyZCBjb2RlIGlzIGdv aW5nIHRvIGJlIGRlbGV0ZWQuCgo+ID4rI2RlZmluZSB0b19hZDU4MjBfZGV2aWNlKHNkKQljb250 YWluZXJfb2Yoc2QsIHN0cnVjdCBhZDU4MjBfZGV2aWNlLCBzdWJkZXYpCj4gPisKPiA+K3N0cnVj dCBhZDU4MjBfZGV2aWNlIHsKPiA+KwlzdHJ1Y3QgdjRsMl9zdWJkZXYgc3ViZGV2Owo+ID4rCXN0 cnVjdCBhZDU4MjBfcGxhdGZvcm1fZGF0YSAqcGxhdGZvcm1fZGF0YTsKPiA+KwlzdHJ1Y3QgcmVn dWxhdG9yICp2YW5hOwo+ID4rCj4gPisJc3RydWN0IHY0bDJfY3RybF9oYW5kbGVyIGN0cmxzOwo+ ID4rCXUzMiBmb2N1c19hYnNvbHV0ZTsKPiA+Kwl1MzIgZm9jdXNfcmFtcF90aW1lOwo+ID4rCXUz MiBmb2N1c19yYW1wX21vZGU7Cj4gPisKPiA+KwlzdHJ1Y3QgbXV0ZXggcG93ZXJfbG9jazsKPiA+ KwlpbnQgcG93ZXJfY291bnQ7Cj4gPisKPiA+KwlpbnQgc3RhbmRieSA6IDE7Cj4gPit9Owo+ID4r Cj4gCj4gVGhlIHNhbWUgZm9yIHN0cnVjdCBhZDU4MjBfZGV2aWNlLCBpcyBpdCByZWFsbHkgcGFy dCBvZiB0aGUgcHVibGljIEFQST8KClllcywgdGhpcyBpcyBhbHNvIHByaXZhdGUgZm9yIGFkNTgy MC5jCgo+ID4rI2VuZGlmIC8qIEFENTgyMF9IICovCj4gPgo+ID4KPiA+CgotLSAKUGFsaSBSb2jD oXIKcGFsaS5yb2hhckBnbWFpbC5jb20KCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fCmxpbnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1r ZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWls bWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJuZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: pali.rohar@gmail.com (Pali =?utf-8?B?Um9ow6Fy?=) Date: Mon, 23 May 2016 09:41:32 +0200 Subject: [PATCHv3] support for AD5820 camera auto-focus coil In-Reply-To: <574049EF.2090208@gmail.com> References: <20160517181927.GA28741@amd> <20160521054336.GA27123@amd> <573FFF51.1000004@gmail.com> <20160521105607.GA20071@amd> <574049EF.2090208@gmail.com> Message-ID: <20160523074132.GD29844@pali> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Saturday 21 May 2016 14:43:43 Ivaylo Dimitrov wrote: > >diff --git a/include/media/ad5820.h b/include/media/ad5820.h > >new file mode 100644 > >index 0000000..f5a1565 > >--- /dev/null > >+++ b/include/media/ad5820.h > >@@ -0,0 +1,70 @@ > >+/* > >+ * include/media/ad5820.h > >+ * > >+ * Copyright (C) 2008 Nokia Corporation > >+ * Copyright (C) 2007 Texas Instruments > >+ * > >+ * Contact: Tuukka Toivonen > >+ * Sakari Ailus > >+ * > >+ * Based on af_d88.c by Texas Instruments. > >+ * > >+ * This program is free software; you can redistribute it and/or > >+ * modify it under the terms of the GNU General Public License > >+ * version 2 as published by the Free Software Foundation. > >+ * > >+ * This program is distributed in the hope that it will be useful, but > >+ * WITHOUT ANY WARRANTY; without even the implied warranty of > >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >+ * General Public License for more details. > >+ * > >+ * You should have received a copy of the GNU General Public License > >+ * along with this program; if not, write to the Free Software > >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA > >+ * 02110-1301 USA > >+ */ > >+ > >+#ifndef AD5820_H > >+#define AD5820_H > >+ > >+#include > >+#include > >+#include > >+ > >+#include > >+#include > >+ > >+struct regulator; > >+ > >+#define AD5820_NAME "ad5820" > >+#define AD5820_I2C_ADDR (0x18 >> 1) Maybe write I2C address is more readable form? What is reason such bit shift format? > >+/* Register definitions */ > >+#define AD5820_POWER_DOWN (1 << 15) > >+#define AD5820_DAC_SHIFT 4 > > Do those defines really belong here? Isn't it better if they are moved to > ad5820.c? For me it looks like this is private for ad5820.c. > >+#define AD5820_RAMP_MODE_LINEAR (0 << 3) > >+#define AD5820_RAMP_MODE_64_16 (1 << 3) > >+ > >+struct ad5820_platform_data { > >+ int (*set_xshutdown)(struct v4l2_subdev *subdev, int set); > >+}; This is for legacy board code support right? We need DT support for N900 as legacy board code is going to be deleted. > >+#define to_ad5820_device(sd) container_of(sd, struct ad5820_device, subdev) > >+ > >+struct ad5820_device { > >+ struct v4l2_subdev subdev; > >+ struct ad5820_platform_data *platform_data; > >+ struct regulator *vana; > >+ > >+ struct v4l2_ctrl_handler ctrls; > >+ u32 focus_absolute; > >+ u32 focus_ramp_time; > >+ u32 focus_ramp_mode; > >+ > >+ struct mutex power_lock; > >+ int power_count; > >+ > >+ int standby : 1; > >+}; > >+ > > The same for struct ad5820_device, is it really part of the public API? Yes, this is also private for ad5820.c > >+#endif /* AD5820_H */ > > > > > > -- Pali Roh?r pali.rohar at gmail.com From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:35099 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753165AbcEWHlg (ORCPT ); Mon, 23 May 2016 03:41:36 -0400 Date: Mon, 23 May 2016 09:41:32 +0200 From: Pali =?utf-8?B?Um9ow6Fy?= To: Pavel Machek Cc: Ivaylo Dimitrov , sre@kernel.org, kernel list , linux-arm-kernel , linux-omap@vger.kernel.org, tony@atomide.com, khilman@kernel.org, aaro.koskinen@iki.fi, patrikbachan@gmail.com, serge@hallyn.com, linux-media@vger.kernel.org, mchehab@osg.samsung.com, sakari.ailus@iki.fi Subject: Re: [PATCHv3] support for AD5820 camera auto-focus coil Message-ID: <20160523074132.GD29844@pali> References: <20160517181927.GA28741@amd> <20160521054336.GA27123@amd> <573FFF51.1000004@gmail.com> <20160521105607.GA20071@amd> <574049EF.2090208@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <574049EF.2090208@gmail.com> Sender: linux-media-owner@vger.kernel.org List-ID: On Saturday 21 May 2016 14:43:43 Ivaylo Dimitrov wrote: > >diff --git a/include/media/ad5820.h b/include/media/ad5820.h > >new file mode 100644 > >index 0000000..f5a1565 > >--- /dev/null > >+++ b/include/media/ad5820.h > >@@ -0,0 +1,70 @@ > >+/* > >+ * include/media/ad5820.h > >+ * > >+ * Copyright (C) 2008 Nokia Corporation > >+ * Copyright (C) 2007 Texas Instruments > >+ * > >+ * Contact: Tuukka Toivonen > >+ * Sakari Ailus > >+ * > >+ * Based on af_d88.c by Texas Instruments. > >+ * > >+ * This program is free software; you can redistribute it and/or > >+ * modify it under the terms of the GNU General Public License > >+ * version 2 as published by the Free Software Foundation. > >+ * > >+ * This program is distributed in the hope that it will be useful, but > >+ * WITHOUT ANY WARRANTY; without even the implied warranty of > >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >+ * General Public License for more details. > >+ * > >+ * You should have received a copy of the GNU General Public License > >+ * along with this program; if not, write to the Free Software > >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA > >+ * 02110-1301 USA > >+ */ > >+ > >+#ifndef AD5820_H > >+#define AD5820_H > >+ > >+#include > >+#include > >+#include > >+ > >+#include > >+#include > >+ > >+struct regulator; > >+ > >+#define AD5820_NAME "ad5820" > >+#define AD5820_I2C_ADDR (0x18 >> 1) Maybe write I2C address is more readable form? What is reason such bit shift format? > >+/* Register definitions */ > >+#define AD5820_POWER_DOWN (1 << 15) > >+#define AD5820_DAC_SHIFT 4 > > Do those defines really belong here? Isn't it better if they are moved to > ad5820.c? For me it looks like this is private for ad5820.c. > >+#define AD5820_RAMP_MODE_LINEAR (0 << 3) > >+#define AD5820_RAMP_MODE_64_16 (1 << 3) > >+ > >+struct ad5820_platform_data { > >+ int (*set_xshutdown)(struct v4l2_subdev *subdev, int set); > >+}; This is for legacy board code support right? We need DT support for N900 as legacy board code is going to be deleted. > >+#define to_ad5820_device(sd) container_of(sd, struct ad5820_device, subdev) > >+ > >+struct ad5820_device { > >+ struct v4l2_subdev subdev; > >+ struct ad5820_platform_data *platform_data; > >+ struct regulator *vana; > >+ > >+ struct v4l2_ctrl_handler ctrls; > >+ u32 focus_absolute; > >+ u32 focus_ramp_time; > >+ u32 focus_ramp_mode; > >+ > >+ struct mutex power_lock; > >+ int power_count; > >+ > >+ int standby : 1; > >+}; > >+ > > The same for struct ad5820_device, is it really part of the public API? Yes, this is also private for ad5820.c > >+#endif /* AD5820_H */ > > > > > > -- Pali Rohár pali.rohar@gmail.com