Message ID | 1527242135-22866-1-git-send-email-bingbu.cao@intel.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [RESEND,V2,1/2] dt-bindings: Add bindings for AKM ak7375 voice coil lens | expand |
On Fri, May 25, 2018 at 05:55:34PM +0800, bingbu.cao@intel.com wrote: > From: Bingbu Cao <bingbu.cao@intel.com> > > Add device tree bindings for AKM ak7375 voice coil lens > driver. This chip is used to drive a lens in a camera module. > > Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com> > Signed-off-by: Bingbu Cao <bingbu.cao@intel.com> > --- > Changes since v1: > - remove the vendor prefix change > --- > Documentation/devicetree/bindings/media/i2c/ak7375.txt | 8 ++++++++ > 1 file changed, 8 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/ak7375.txt Reviewed-by: Rob Herring <robh@kernel.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bingbu, A few comments below. On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote: > From: Bingbu Cao <bingbu.cao@intel.com> > > Add a V4L2 sub-device driver for the ak7375 lens voice coil. > This is a voice coil module using the I2C bus to control the > focus position. > > Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com> > Signed-off-by: Bingbu Cao <bingbu.cao@intel.com> > --- > MAINTAINERS | 8 ++ > drivers/media/i2c/Kconfig | 10 ++ > drivers/media/i2c/Makefile | 1 + > drivers/media/i2c/ak7375.c | 278 +++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 297 insertions(+) > create mode 100644 drivers/media/i2c/ak7375.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ea362219c4aa..20379a7baca0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -625,6 +625,14 @@ T: git git://linuxtv.org/anttip/media_tree.git > S: Maintained > F: drivers/media/usb/airspy/ > > +AKM AK7375 LENS VOICE COIL DRIVER > +M: Tianshu Qiu <tian.shu.qiu@intel.com> > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: drivers/media/i2c/ak7375.c > +F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt The name of the file also needs to match. Currently it doesn't. How about "asahi-kasei,ak7375.txt"? Could you also move the MAINTAINERS entry to the patch adding the DT bindings? > + > ALACRITECH GIGABIT ETHERNET DRIVER > M: Lino Sanfilippo <LinoSanfilippo@gmx.de> > S: Maintained > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index 341452fe98df..ff3cb5afb0e1 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -326,6 +326,16 @@ config VIDEO_AD5820 > This is a driver for the AD5820 camera lens voice coil. > It is used for example in Nokia N900 (RX-51). > > +config VIDEO_AK7375 > + tristate "AK7375 lens voice coil support" > + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER > + depends on VIDEO_V4L2_SUBDEV_API > + help > + This is a driver for the AK7375 camera lens voice coil. > + AK7375 is a 12 bit DAC with 120mA output current sink > + capability. This is designed for linear control of > + voice coil motors, controlled via I2C serial interface. > + > config VIDEO_DW9714 > tristate "DW9714 lens voice coil support" > depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER > diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile > index d679d57cd3b3..05b97e319ea9 100644 > --- a/drivers/media/i2c/Makefile > +++ b/drivers/media/i2c/Makefile > @@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o > obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o > obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o > obj-$(CONFIG_VIDEO_AD5820) += ad5820.o > +obj-$(CONFIG_VIDEO_AK7375) += ak7375.o > obj-$(CONFIG_VIDEO_DW9714) += dw9714.o > obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o > obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o > diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c > new file mode 100644 > index 000000000000..012e673e9ced > --- /dev/null > +++ b/drivers/media/i2c/ak7375.c ... > +static const struct of_device_id ak7375_of_table[] = { > + { .compatible = "akm,ak7375" }, "asahi-kasei,ak7375"
On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote: > +static int ak7375_i2c_write(struct ak7375_device *ak7375, > + u8 addr, u16 data, int size) > +{ > + struct i2c_client *client = v4l2_get_subdevdata(&ak7375->sd); > + int ret; > + u8 buf[3]; > + > + if (size != 1 && size != 2) > + return -EINVAL; > + buf[0] = addr; > + buf[2] = data & 0xff; > + if (size == 2) > + buf[1] = data >> 8; > + ret = i2c_master_send(client, (const char *)buf, size + 1); I don't have a data datasheet for this thing, but it looks like buf[1] will be undefined for writes the size of which is 1. And this what appears to be written to the device as well... > + if (ret < 0) > + return ret; > + if (ret != size + 1) > + return -EIO; > + return 0; > +}
On 2018年06月01日 17:34, Sakari Ailus wrote: > Hi Bingbu, > > A few comments below. > > On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote: >> From: Bingbu Cao <bingbu.cao@intel.com> >> >> Add a V4L2 sub-device driver for the ak7375 lens voice coil. >> This is a voice coil module using the I2C bus to control the >> focus position. >> >> Signed-off-by: Tianshu Qiu <tian.shu.qiu@intel.com> >> Signed-off-by: Bingbu Cao <bingbu.cao@intel.com> >> --- >> MAINTAINERS | 8 ++ >> drivers/media/i2c/Kconfig | 10 ++ >> drivers/media/i2c/Makefile | 1 + >> drivers/media/i2c/ak7375.c | 278 +++++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 297 insertions(+) >> create mode 100644 drivers/media/i2c/ak7375.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index ea362219c4aa..20379a7baca0 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -625,6 +625,14 @@ T: git git://linuxtv.org/anttip/media_tree.git >> S: Maintained >> F: drivers/media/usb/airspy/ >> >> +AKM AK7375 LENS VOICE COIL DRIVER >> +M: Tianshu Qiu <tian.shu.qiu@intel.com> >> +L: linux-media@vger.kernel.org >> +T: git git://linuxtv.org/media_tree.git >> +S: Maintained >> +F: drivers/media/i2c/ak7375.c >> +F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt > The name of the file also needs to match. Currently it doesn't. How about > "asahi-kasei,ak7375.txt"? Ack. Does it make sense if just change the compatible string to "asahi-kasei,ak7375" and keep the file name unchanged? I have no clear idea about the DT binding rules. > > Could you also move the MAINTAINERS entry to the patch adding the DT > bindings? Ack. > >> + >> ALACRITECH GIGABIT ETHERNET DRIVER >> M: Lino Sanfilippo <LinoSanfilippo@gmx.de> >> S: Maintained >> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig >> index 341452fe98df..ff3cb5afb0e1 100644 >> --- a/drivers/media/i2c/Kconfig >> +++ b/drivers/media/i2c/Kconfig >> @@ -326,6 +326,16 @@ config VIDEO_AD5820 >> This is a driver for the AD5820 camera lens voice coil. >> It is used for example in Nokia N900 (RX-51). >> >> +config VIDEO_AK7375 >> + tristate "AK7375 lens voice coil support" >> + depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER >> + depends on VIDEO_V4L2_SUBDEV_API >> + help >> + This is a driver for the AK7375 camera lens voice coil. >> + AK7375 is a 12 bit DAC with 120mA output current sink >> + capability. This is designed for linear control of >> + voice coil motors, controlled via I2C serial interface. >> + >> config VIDEO_DW9714 >> tristate "DW9714 lens voice coil support" >> depends on I2C && VIDEO_V4L2 && MEDIA_CONTROLLER >> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile >> index d679d57cd3b3..05b97e319ea9 100644 >> --- a/drivers/media/i2c/Makefile >> +++ b/drivers/media/i2c/Makefile >> @@ -23,6 +23,7 @@ obj-$(CONFIG_VIDEO_SAA7127) += saa7127.o >> obj-$(CONFIG_VIDEO_SAA7185) += saa7185.o >> obj-$(CONFIG_VIDEO_SAA6752HS) += saa6752hs.o >> obj-$(CONFIG_VIDEO_AD5820) += ad5820.o >> +obj-$(CONFIG_VIDEO_AK7375) += ak7375.o >> obj-$(CONFIG_VIDEO_DW9714) += dw9714.o >> obj-$(CONFIG_VIDEO_ADV7170) += adv7170.o >> obj-$(CONFIG_VIDEO_ADV7175) += adv7175.o >> diff --git a/drivers/media/i2c/ak7375.c b/drivers/media/i2c/ak7375.c >> new file mode 100644 >> index 000000000000..012e673e9ced >> --- /dev/null >> +++ b/drivers/media/i2c/ak7375.c > ... > >> +static const struct of_device_id ak7375_of_table[] = { >> + { .compatible = "akm,ak7375" }, > "asahi-kasei,ak7375" > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jun 01, 2018 at 05:54:30PM +0800, Bing Bu Cao wrote: > > > +AKM AK7375 LENS VOICE COIL DRIVER > > > +M: Tianshu Qiu <tian.shu.qiu@intel.com> > > > +L: linux-media@vger.kernel.org > > > +T: git git://linuxtv.org/media_tree.git > > > +S: Maintained > > > +F: drivers/media/i2c/ak7375.c > > > +F: Documentation/devicetree/bindings/media/i2c/akm,ak7375.txt > > The name of the file also needs to match. Currently it doesn't. How about > > "asahi-kasei,ak7375.txt"? > Ack. > Does it make sense if just change the compatible string to > "asahi-kasei,ak7375" and keep the file name unchanged? Most binding files in the directory seem to use the chip name rather than the full compatible string (including the vendor). I guess both are fine.
On 2018年06月01日 17:42, Sakari Ailus wrote: > On Fri, May 25, 2018 at 05:55:35PM +0800, bingbu.cao@intel.com wrote: >> +static int ak7375_i2c_write(struct ak7375_device *ak7375, >> + u8 addr, u16 data, int size) >> +{ >> + struct i2c_client *client = v4l2_get_subdevdata(&ak7375->sd); >> + int ret; >> + u8 buf[3]; >> + >> + if (size != 1 && size != 2) >> + return -EINVAL; >> + buf[0] = addr; >> + buf[2] = data & 0xff; >> + if (size == 2) >> + buf[1] = data >> 8; >> + ret = i2c_master_send(client, (const char *)buf, size + 1); > I don't have a data datasheet for this thing, but it looks like buf[1] will > be undefined for writes the size of which is 1. And this what appears to be > written to the device as well... I check the datasheet once again and find out that the logic here for size==1 is not correct, I will change it in next version. Thanks. Here is the write section in datasheet: ------ After receiving the second byte (register address), AK7375 generates an acknowledge then receives the third byte. The third and the following bytes represent control data. Control data consists of 8 bits and is based on the MSB-first configuration. AK7375 can write multiple bytes of data at a time. After reception of the third byte (control data), AK7375 generates an acknowledge then receives the next data. If additional data is received instead of the stop condition after receiving one byte of data, the address counter inside the LSI chip is automatically incremented and the data is written at the next address. ------ > >> + if (ret < 0) >> + return ret; >> + if (ret != size + 1) >> + return -EIO; >> + return 0; >> +} -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/media/i2c/ak7375.txt b/Documentation/devicetree/bindings/media/i2c/ak7375.txt new file mode 100644 index 000000000000..aa3e24b41241 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ak7375.txt @@ -0,0 +1,8 @@ +Asahi Kasei Microdevices AK7375 voice coil lens driver + +AK7375 is a camera voice coil lens. + +Mandatory properties: + +- compatible: "asahi-kasei,ak7375" +- reg: I2C slave address