diff mbox

[PATCHv5] support for AD5820 camera auto-focus coil

Message ID 20160601220840.GA21946@amd (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Machek June 1, 2016, 10:08 p.m. UTC
On Wed 2016-06-01 18:24:39, Sakari Ailus wrote:
> Hi Pavel,

> > Well, it does not use any dt properties. So there's not really much to
> > discuss with dt people...
> > 
> > Maybe "ad5820" needs to go to list of simple i2c drivers somewhere,
> > but...
> 
> It's an I2C device and it does use a regulator. Not a lot, though, these are
> both quite basic stuff. This should still be documented as the people who
> write the DT bindings (in general) aren't expected to read driver code as
> well. That's at least my understanding.

Yep, you are right, I forgot about the regulator. Something like this?

Thanks,
									Pavel

Comments

Sakari Ailus June 2, 2016, 7:45 a.m. UTC | #1
On Thu, Jun 02, 2016 at 12:08:40AM +0200, Pavel Machek wrote:
> On Wed 2016-06-01 18:24:39, Sakari Ailus wrote:
> > Hi Pavel,
> 
> > > Well, it does not use any dt properties. So there's not really much to
> > > discuss with dt people...
> > > 
> > > Maybe "ad5820" needs to go to list of simple i2c drivers somewhere,
> > > but...
> > 
> > It's an I2C device and it does use a regulator. Not a lot, though, these are
> > both quite basic stuff. This should still be documented as the people who
> > write the DT bindings (in general) aren't expected to read driver code as
> > well. That's at least my understanding.
> 
> Yep, you are right, I forgot about the regulator. Something like this?
> 
> Thanks,
> 									Pavel
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> new file mode 100644
> index 0000000..87c98f1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt

I might use the compatible string as such as a part of the file name. Up to
you.

> @@ -0,0 +1,20 @@
> +* Analog Devices AD5820 autofocus coil
> +
> +Required Properties:
> +
> +  - compatible: Must contain "adi,ad5820"
> +
> +  - reg: I2C slave address
> +
> +  - VANA-supply: supply of voltage for VANA pin
> +
> +Example:
> +
> +       /* D/A converter for auto-focus */

There is definitely D/A conversion happening there but I'm not sure I'd
characterise the device as such. They're typically called "voice coil
drivers", perhaps because the devices are similar to a parts of a
loudspeaker.

> +       ad5820: dac@0c {
> +               compatible = "adi,ad5820";
> +               reg = <0x0c>;
> +
> +               VANA-supply = <&vaux4>;
> +       };
> +
> 
>
Pavel Machek June 2, 2016, 7:27 p.m. UTC | #2
On Thu 2016-06-02 10:45:45, Sakari Ailus wrote:
> On Thu, Jun 02, 2016 at 12:08:40AM +0200, Pavel Machek wrote:
> > On Wed 2016-06-01 18:24:39, Sakari Ailus wrote:
> > > Hi Pavel,
> > 
> > > > Well, it does not use any dt properties. So there's not really much to
> > > > discuss with dt people...
> > > > 
> > > > Maybe "ad5820" needs to go to list of simple i2c drivers somewhere,
> > > > but...
> > > 
> > > It's an I2C device and it does use a regulator. Not a lot, though, these are
> > > both quite basic stuff. This should still be documented as the people who
> > > write the DT bindings (in general) aren't expected to read driver code as
> > > well. That's at least my understanding.
> > 
> > Yep, you are right, I forgot about the regulator. Something like this?
> > 
> > Thanks,
> > 									Pavel
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > new file mode 100644
> > index 0000000..87c98f1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> 
> I might use the compatible string as such as a part of the file name. Up to
> you.

Sorry, can't do that, for consistency with other ad* files in the directory.

> > @@ -0,0 +1,20 @@
> > +* Analog Devices AD5820 autofocus coil
> > +
> > +Required Properties:
> > +
> > +  - compatible: Must contain "adi,ad5820"
> > +
> > +  - reg: I2C slave address
> > +
> > +  - VANA-supply: supply of voltage for VANA pin
> > +
> > +Example:
> > +
> > +       /* D/A converter for auto-focus */
> 
> There is definitely D/A conversion happening there but I'm not sure I'd
> characterise the device as such. They're typically called "voice coil
> drivers", perhaps because the devices are similar to a parts of a
> loudspeaker.

Well, I'm pretty sure I did not invent that comment, but I can drop
it.

Best regards,
								Pavel
Sakari Ailus June 2, 2016, 9:23 p.m. UTC | #3
On Thu, Jun 02, 2016 at 09:27:37PM +0200, Pavel Machek wrote:
> On Thu 2016-06-02 10:45:45, Sakari Ailus wrote:
> > On Thu, Jun 02, 2016 at 12:08:40AM +0200, Pavel Machek wrote:
> > > On Wed 2016-06-01 18:24:39, Sakari Ailus wrote:
> > > > Hi Pavel,
> > > 
> > > > > Well, it does not use any dt properties. So there's not really much to
> > > > > discuss with dt people...
> > > > > 
> > > > > Maybe "ad5820" needs to go to list of simple i2c drivers somewhere,
> > > > > but...
> > > > 
> > > > It's an I2C device and it does use a regulator. Not a lot, though, these are
> > > > both quite basic stuff. This should still be documented as the people who
> > > > write the DT bindings (in general) aren't expected to read driver code as
> > > > well. That's at least my understanding.
> > > 
> > > Yep, you are right, I forgot about the regulator. Something like this?
> > > 
> > > Thanks,
> > > 									Pavel
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > > new file mode 100644
> > > index 0000000..87c98f1
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
> > 
> > I might use the compatible string as such as a part of the file name. Up to
> > you.
> 
> Sorry, can't do that, for consistency with other ad* files in the directory.
> 
> > > @@ -0,0 +1,20 @@
> > > +* Analog Devices AD5820 autofocus coil
> > > +
> > > +Required Properties:
> > > +
> > > +  - compatible: Must contain "adi,ad5820"
> > > +
> > > +  - reg: I2C slave address
> > > +
> > > +  - VANA-supply: supply of voltage for VANA pin
> > > +
> > > +Example:
> > > +
> > > +       /* D/A converter for auto-focus */
> > 
> > There is definitely D/A conversion happening there but I'm not sure I'd
> > characterise the device as such. They're typically called "voice coil
> > drivers", perhaps because the devices are similar to a parts of a
> > loudspeaker.
> 
> Well, I'm pretty sure I did not invent that comment, but I can drop
> it.

Both are fine for me.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/ad5820.txt b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
new file mode 100644
index 0000000..87c98f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ad5820.txt
@@ -0,0 +1,20 @@ 
+* Analog Devices AD5820 autofocus coil
+
+Required Properties:
+
+  - compatible: Must contain "adi,ad5820"
+
+  - reg: I2C slave address
+
+  - VANA-supply: supply of voltage for VANA pin
+
+Example:
+
+       /* D/A converter for auto-focus */
+       ad5820: dac@0c {
+               compatible = "adi,ad5820";
+               reg = <0x0c>;
+
+               VANA-supply = <&vaux4>;
+       };
+