| Message ID | 1519402422-9595-3-git-send-email-andy.yeh@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> wrote: > From: Alan Chiang <alanx.chiang@intel.com> > > Dongwoon DW9807 is a voice coil lens driver. > > Also add a vendor prefix for Dongwoon for one did not exist previously. Where's that? > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > --- > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +++++++++ DACs generally go in bindings/iio/dac/ > 1 file changed, 9 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt > > diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt > new file mode 100644 > index 0000000..0a1a860 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt > @@ -0,0 +1,9 @@ > +Dongwoon Anatech DW9807 voice coil lens driver > + > +DW9807 is a 10-bit DAC with current sink capability. It is intended for > +controlling voice coil lenses. > + > +Mandatory properties: > + > +- compatible: "dongwoon,dw9807" > +- reg: I2C slave address > -- > 2.7.4 > > -- > 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 Rob, Thanks for the review. On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> wrote: > > From: Alan Chiang <alanx.chiang@intel.com> > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > Where's that? Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't relevant indeed and should be removed. > > > > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > > --- > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +++++++++ > > DACs generally go in bindings/iio/dac/ We have quite a few lens voice coil drivers under bindings/media/i2c now. I don't really object to putting this one to bindings/iio/dac but then the rest should be moved as well. The camera LED flash drivers are under bindings/leds so this would actually be analoguous to that. The lens voice coil drivers are perhaps still a bit more bound to the domain (camera) than the LED flash drivers. I can send a patch if you think the existing bindings should be moved; let me know.
On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > Hi Rob, > > Thanks for the review. > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> wrote: > > > From: Alan Chiang <alanx.chiang@intel.com> > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > > > Where's that? > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > relevant indeed and should be removed. > > > > > > > > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > > > --- > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +++++++++ > > > > DACs generally go in bindings/iio/dac/ > > We have quite a few lens voice coil drivers under bindings/media/i2c now. I > don't really object to putting this one to bindings/iio/dac but then the > rest should be moved as well. > > The camera LED flash drivers are under bindings/leds so this would actually > be analoguous to that. The lens voice coil drivers are perhaps still a bit > more bound to the domain (camera) than the LED flash drivers. The h/w is bound to that function or just the s/w? > I can send a patch if you think the existing bindings should be moved; let > me know. I'm okay if they are separate as long as we're not going to see the same device show up in both places. However, "i2c" is not the best directory choice. It should be by function, so we can find common properties. Rob
On Fri, Mar 02, 2018 at 12:59:00PM -0600, Rob Herring wrote: > On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > > Hi Rob, > > > > Thanks for the review. > > > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> wrote: > > > > From: Alan Chiang <alanx.chiang@intel.com> > > > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > > > > > Where's that? > > > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > > relevant indeed and should be removed. > > > > > > > > > > > > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > > > > --- > > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +++++++++ > > > > > > DACs generally go in bindings/iio/dac/ > > > > We have quite a few lens voice coil drivers under bindings/media/i2c now. I > > don't really object to putting this one to bindings/iio/dac but then the > > rest should be moved as well. > > > > The camera LED flash drivers are under bindings/leds so this would actually > > be analoguous to that. The lens voice coil drivers are perhaps still a bit > > more bound to the domain (camera) than the LED flash drivers. > > The h/w is bound to that function or just the s/w? The hardware. I guess in principle you could use them for other purposes --- most devices seem to be current sinks with configurable current --- but I've never seen that. The datasheet (dw9714) is here: <URL:http://www.datasheetspdf.com/datasheet/download.php?id=840322> > > > I can send a patch if you think the existing bindings should be moved; let > > me know. > > I'm okay if they are separate as long as we're not going to see the > same device show up in both places. However, "i2c" is not the best Ack. I wouldn't expect that. The datasheets of such devices clearly label the devices voice coil module drivers. > directory choice. It should be by function, so we can find common > properties. I2c devices in the media subsystem tend to be peripherals that are always used with another device with access to some system bus. Camera sensors, lens devices and tuners can be found there currently. I don't know the original reasoning but it most likely is related to that. In terms of different kinds of devices we have currently at least the following: Camera ISPs and CSI-2 receivers Video muxes Video codecs Camera sensors Camera lens drivers (right now only voice coil modules?) Tuners (DVB, radio, analogue TV, whatever) Radio transmitters HDMI CEC Remote controllers JPEG codecs Cc Hans, too.
On Sat, Mar 3, 2018 at 5:15 AM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > On Fri, Mar 02, 2018 at 12:59:00PM -0600, Rob Herring wrote: > > On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > > > Hi Rob, > > > > > > Thanks for the review. > > > > > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> wrote: > > > > > From: Alan Chiang <alanx.chiang@intel.com> > > > > > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > > > > > Also add a vendor prefix for Dongwoon for one did not exist previously. > > > > > > > > Where's that? > > > > > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > > > relevant indeed and should be removed. > > > > > > > > > > > > > > > > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > > > > > --- > > > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt | 9 +++++++++ > > > > > > > > DACs generally go in bindings/iio/dac/ > > > > > > We have quite a few lens voice coil drivers under bindings/media/i2c now. I > > > don't really object to putting this one to bindings/iio/dac but then the > > > rest should be moved as well. > > > > > > The camera LED flash drivers are under bindings/leds so this would actually > > > be analoguous to that. The lens voice coil drivers are perhaps still a bit > > > more bound to the domain (camera) than the LED flash drivers. > > > > The h/w is bound to that function or just the s/w? > The hardware. I guess in principle you could use them for other purposes > --- most devices seem to be current sinks with configurable current --- but > I've never seen that. > The datasheet (dw9714) is here: > <URL:http://www.datasheetspdf.com/datasheet/download.php?id=840322> > > > > > I can send a patch if you think the existing bindings should be moved; let > > > me know. > > > > I'm okay if they are separate as long as we're not going to see the > > same device show up in both places. However, "i2c" is not the best > Ack. I wouldn't expect that. The datasheets of such devices clearly label > the devices voice coil module drivers. > > directory choice. It should be by function, so we can find common > > properties. > I2c devices in the media subsystem tend to be peripherals that are always > used with another device with access to some system bus. Camera sensors, lens > devices and tuners can be found there currently. I don't know the original > reasoning but it most likely is related to that. > In terms of different kinds of devices we have currently at least the > following: > Camera ISPs and CSI-2 receivers > Video muxes > Video codecs > Camera sensors > Camera lens drivers (right now only voice coil modules?) > Tuners (DVB, radio, analogue TV, whatever) > Radio transmitters > HDMI CEC > Remote controllers > JPEG codecs > Cc Hans, too. Any updates here? To be honest, I'm not sure there is too much to be thinking about here. This particular hardware block is a lens driver, specifically designed to be used with cameras. Quoting maker's website [1]: "Driver ICs for automatically focus on images of mobile cameras. Dongwoon Anatech's AF driver ICs are optimized mobile cameras with low power, low noise, smallest package, as well as include various lens position control methodology." IMHO putting its bindings under the more general purpose iio/ directory doesn't make much sense and would be actually confusing. [1] http://www.dwanatech.com/eng/sub/sub02_01.php?cat_no=6 Best regards, Tomasz
Hi Tomasz, On Thu, Apr 05, 2018 at 08:21:56AM +0000, Tomasz Figa wrote: > On Sat, Mar 3, 2018 at 5:15 AM Sakari Ailus <sakari.ailus@linux.intel.com> > wrote: > > > On Fri, Mar 02, 2018 at 12:59:00PM -0600, Rob Herring wrote: > > > On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > > > > Hi Rob, > > > > > > > > Thanks for the review. > > > > > > > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > > > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> > wrote: > > > > > > From: Alan Chiang <alanx.chiang@intel.com> > > > > > > > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > > > > > > > Also add a vendor prefix for Dongwoon for one did not exist > previously. > > > > > > > > > > Where's that? > > > > > > > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > > > > relevant indeed and should be removed. > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > > > > > > --- > > > > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt > | 9 +++++++++ > > > > > > > > > > DACs generally go in bindings/iio/dac/ > > > > > > > > We have quite a few lens voice coil drivers under bindings/media/i2c > now. I > > > > don't really object to putting this one to bindings/iio/dac but then > the > > > > rest should be moved as well. > > > > > > > > The camera LED flash drivers are under bindings/leds so this would > actually > > > > be analoguous to that. The lens voice coil drivers are perhaps still > a bit > > > > more bound to the domain (camera) than the LED flash drivers. > > > > > > The h/w is bound to that function or just the s/w? > > > The hardware. I guess in principle you could use them for other purposes > > --- most devices seem to be current sinks with configurable current --- > but > > I've never seen that. > > > The datasheet (dw9714) is here: > > > <URL:http://www.datasheetspdf.com/datasheet/download.php?id=840322> > > > > > > > > I can send a patch if you think the existing bindings should be > moved; let > > > > me know. > > > > > > I'm okay if they are separate as long as we're not going to see the > > > same device show up in both places. However, "i2c" is not the best > > > Ack. I wouldn't expect that. The datasheets of such devices clearly label > > the devices voice coil module drivers. > > > > directory choice. It should be by function, so we can find common > > > properties. > > > I2c devices in the media subsystem tend to be peripherals that are always > > used with another device with access to some system bus. Camera sensors, > lens > > devices and tuners can be found there currently. I don't know the original > > reasoning but it most likely is related to that. > > > In terms of different kinds of devices we have currently at least the > > following: > > > Camera ISPs and CSI-2 receivers > > Video muxes > > Video codecs > > Camera sensors > > Camera lens drivers (right now only voice coil modules?) > > Tuners (DVB, radio, analogue TV, whatever) > > Radio transmitters > > HDMI CEC > > Remote controllers > > JPEG codecs > > > Cc Hans, too. > > Any updates here? > > To be honest, I'm not sure there is too much to be thinking about here. > This particular hardware block is a lens driver, specifically designed to > be used with cameras. Quoting maker's website [1]: > > "Driver ICs for automatically focus on images of mobile cameras. > Dongwoon Anatech's AF driver ICs are optimized mobile cameras > with low power, low noise, smallest package, as well as include > various lens position control methodology." > > IMHO putting its bindings under the more general purpose iio/ directory > doesn't make much sense and would be actually confusing. > > [1] http://www.dwanatech.com/eng/sub/sub02_01.php?cat_no=6 Rob has acked v6 Andy sent some time ago while the driver patch has unaddressed comments from Jacopo. I think Andy (unintentionally) missed you from cc list: <URL:https://www.spinics.net/lists/linux-media/msg130709.html>
Hi Sakari, On Thu, Apr 5, 2018 at 6:26 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > Hi Tomasz, > On Thu, Apr 05, 2018 at 08:21:56AM +0000, Tomasz Figa wrote: > > On Sat, Mar 3, 2018 at 5:15 AM Sakari Ailus < sakari.ailus@linux.intel.com> > > wrote: > > > > > On Fri, Mar 02, 2018 at 12:59:00PM -0600, Rob Herring wrote: > > > > On Wed, Feb 28, 2018 at 03:31:26PM +0200, Sakari Ailus wrote: > > > > > Hi Rob, > > > > > > > > > > Thanks for the review. > > > > > > > > > > On Tue, Feb 27, 2018 at 04:10:31PM -0600, Rob Herring wrote: > > > > > > On Fri, Feb 23, 2018 at 10:13 AM, Andy Yeh <andy.yeh@intel.com> > > wrote: > > > > > > > From: Alan Chiang <alanx.chiang@intel.com> > > > > > > > > > > > > > > Dongwoon DW9807 is a voice coil lens driver. > > > > > > > > > > > > > > Also add a vendor prefix for Dongwoon for one did not exist > > previously. > > > > > > > > > > > > Where's that? > > > > > > > > > > Added by aece98a912d92444ea9da03b04269407d1308f1f . So that line isn't > > > > > relevant indeed and should be removed. > > > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Andy Yeh <andy.yeh@intel.com> > > > > > > > --- > > > > > > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt > > | 9 +++++++++ > > > > > > > > > > > > DACs generally go in bindings/iio/dac/ > > > > > > > > > > We have quite a few lens voice coil drivers under bindings/media/i2c > > now. I > > > > > don't really object to putting this one to bindings/iio/dac but then > > the > > > > > rest should be moved as well. > > > > > > > > > > The camera LED flash drivers are under bindings/leds so this would > > actually > > > > > be analoguous to that. The lens voice coil drivers are perhaps still > > a bit > > > > > more bound to the domain (camera) than the LED flash drivers. > > > > > > > > The h/w is bound to that function or just the s/w? > > > > > The hardware. I guess in principle you could use them for other purposes > > > --- most devices seem to be current sinks with configurable current --- > > but > > > I've never seen that. > > > > > The datasheet (dw9714) is here: > > > > > <URL:http://www.datasheetspdf.com/datasheet/download.php?id=840322> > > > > > > > > > > > I can send a patch if you think the existing bindings should be > > moved; let > > > > > me know. > > > > > > > > I'm okay if they are separate as long as we're not going to see the > > > > same device show up in both places. However, "i2c" is not the best > > > > > Ack. I wouldn't expect that. The datasheets of such devices clearly label > > > the devices voice coil module drivers. > > > > > > directory choice. It should be by function, so we can find common > > > > properties. > > > > > I2c devices in the media subsystem tend to be peripherals that are always > > > used with another device with access to some system bus. Camera sensors, > > lens > > > devices and tuners can be found there currently. I don't know the original > > > reasoning but it most likely is related to that. > > > > > In terms of different kinds of devices we have currently at least the > > > following: > > > > > Camera ISPs and CSI-2 receivers > > > Video muxes > > > Video codecs > > > Camera sensors > > > Camera lens drivers (right now only voice coil modules?) > > > Tuners (DVB, radio, analogue TV, whatever) > > > Radio transmitters > > > HDMI CEC > > > Remote controllers > > > JPEG codecs > > > > > Cc Hans, too. > > > > Any updates here? > > > > To be honest, I'm not sure there is too much to be thinking about here. > > This particular hardware block is a lens driver, specifically designed to > > be used with cameras. Quoting maker's website [1]: > > > > "Driver ICs for automatically focus on images of mobile cameras. > > Dongwoon Anatech's AF driver ICs are optimized mobile cameras > > with low power, low noise, smallest package, as well as include > > various lens position control methodology." > > > > IMHO putting its bindings under the more general purpose iio/ directory > > doesn't make much sense and would be actually confusing. > > > > [1] http://www.dwanatech.com/eng/sub/sub02_01.php?cat_no=6 > Rob has acked v6 Andy sent some time ago while the driver patch has > unaddressed comments from Jacopo. I think Andy (unintentionally) missed > you from cc list: > <URL:https://www.spinics.net/lists/linux-media/msg130709.html> That's good to hear. Thanks for the pointer. Best regards, Tomasz
diff --git a/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt new file mode 100644 index 0000000..0a1a860 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/dongwoon,dw9807.txt @@ -0,0 +1,9 @@ +Dongwoon Anatech DW9807 voice coil lens driver + +DW9807 is a 10-bit DAC with current sink capability. It is intended for +controlling voice coil lenses. + +Mandatory properties: + +- compatible: "dongwoon,dw9807" +- reg: I2C slave address