[PATCH] Device tree binding for Avago APDS990X light sensor

[PATCH] Device tree binding for Avago APDS990X light sensor

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]

From: Filip Matijević <filip.matijevic.pz@gmail.com>

This prepares binding for light sensor used in Nokia N9. 

Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
Signed-off-by: Pavel machek <pavel@ucw.cz>

---

Patches to convert APDS990X driver to device tree and to switch to iio
are available.

diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
new file mode 100644
index 0000000..e038146
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
@@ -0,0 +1,39 @@
+Avago APDS990X driver
+
+Required properties:
+- compatible: "avago,apds990x"
+- reg: address on the I2C bus
+- interrupts: external interrupt line number
+- Vdd-supply: power supply for VDD
+- Vled-supply: power supply for LEDA
+- ga: Glass attenuation
+- cf1: Clear channel factor 1
+- irf1: IR channel factor 1
+- cf2: Clear channel factor 2
+- irf2: IR channel factor 2
+- df: Device factor
+- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
+- ppcount: Proximity pulse count
+
+Example (Nokia N9):
+
+	als_ps@39 {
+		compatible = "avago,apds990x";
+		reg = <0x39>;
+
+		interrupt-parent = <&gpio3>;
+		interrupts = <19 10>; /* gpio_83, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_LOW */
+
+		Vdd-supply = <&vaux1>;
+		Vled-supply = <&vbat>;
+
+		ga	= <168834>;
+		cf1	= <4096>;
+		irf1	= <7824>;
+		cf2	= <877>;
+		irf2	= <1575>;
+		df	= <52>;
+
+		pdrive	= <0x2>; /* APDS_IRLED_CURR_25mA */
+		ppcount	= <5>;
+	};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] Device tree binding for Avago APDS990X light sensor

[Sakari Ailus]

Hi Pavel,

Thanks for the patch. Please see my comments below.

On Wed, Dec 27, 2017 at 10:18:28AM +0100, Pavel Machek wrote:
> From: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This prepares binding for light sensor used in Nokia N9. 
> 
> Signed-off-by: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Pavel machek <pavel-+ZI9xUNit7I@public.gmane.org>
> 
> ---
> 
> Patches to convert APDS990X driver to device tree and to switch to iio
> are available.
> 
> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> new file mode 100644
> index 0000000..e038146
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> @@ -0,0 +1,39 @@
> +Avago APDS990X driver
> +
> +Required properties:
> +- compatible: "avago,apds990x"
> +- reg: address on the I2C bus
> +- interrupts: external interrupt line number
> +- Vdd-supply: power supply for VDD
> +- Vled-supply: power supply for LEDA

AFAIK the custom is to use lower case letters for regulator supplies.

> +- ga: Glass attenuation
> +- cf1: Clear channel factor 1
> +- irf1: IR channel factor 1
> +- cf2: Clear channel factor 2
> +- irf2: IR channel factor 2
> +- df: Device factor
> +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
> +- ppcount: Proximity pulse count

Are these device specific? If so, please add the vendor prefix to them.

I might not use short abbreviations such as "df" either. I wonder what
others think.

> +
> +Example (Nokia N9):
> +
> +	als_ps@39 {
> +		compatible = "avago,apds990x";
> +		reg = <0x39>;
> +
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <19 10>; /* gpio_83, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_LOW */
> +
> +		Vdd-supply = <&vaux1>;
> +		Vled-supply = <&vbat>;
> +
> +		ga	= <168834>;
> +		cf1	= <4096>;
> +		irf1	= <7824>;
> +		cf2	= <877>;
> +		irf2	= <1575>;
> +		df	= <52>;
> +
> +		pdrive	= <0x2>; /* APDS_IRLED_CURR_25mA */
> +		ppcount	= <5>;
> +	};
> 

-- 
Kind regards,

Sakari Ailus
sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Device tree binding for Avago APDS990X light sensor

[Filip Matijević]

Hi Sakari,

and thank you for your input - I've added a few comments below.

On 12/27/2017 07:00 PM, Sakari Ailus wrote:
> Hi Pavel,
> 
> Thanks for the patch. Please see my comments below.
> 
> On Wed, Dec 27, 2017 at 10:18:28AM +0100, Pavel Machek wrote:
>> From: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> This prepares binding for light sensor used in Nokia N9. 
>>
>> Signed-off-by: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Pavel machek <pavel-+ZI9xUNit7I@public.gmane.org>
>>
>> ---
>>
>> Patches to convert APDS990X driver to device tree and to switch to iio
>> are available.
>>
>> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> new file mode 100644
>> index 0000000..e038146
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
>> @@ -0,0 +1,39 @@
>> +Avago APDS990X driver
>> +
>> +Required properties:
>> +- compatible: "avago,apds990x"
>> +- reg: address on the I2C bus
>> +- interrupts: external interrupt line number
>> +- Vdd-supply: power supply for VDD
>> +- Vled-supply: power supply for LEDA
> 
> AFAIK the custom is to use lower case letters for regulator supplies.

I've just used the same notation as in current driver. Datasheet calls
those VDD (with DD being in subscript) and LEDA. I see no problem in
changing those to vdd-supply and vled-supply if no one else objects.

> 
>> +- ga: Glass attenuation
>> +- cf1: Clear channel factor 1
>> +- irf1: IR channel factor 1
>> +- cf2: Clear channel factor 2
>> +- irf2: IR channel factor 2
>> +- df: Device factor
>> +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
>> +- ppcount: Proximity pulse count
> 
> Are these device specific? If so, please add the vendor prefix to them.

AFAIK yes - same as before if no one else objects, these will be changed.

> 
> I might not use short abbreviations such as "df" either. I wonder what
> others think.

These are also come from current driver - some of these comes from the
datasheet itself, but not all. For example "ga" and "df" are in there
(so I I would leave those alone), but "irf1" is called "B", "cf2" is
called "C" and "irf2" is called "D" (I guess the missing "cf1" should be
"A", but there is no such thing in the datasheet).
IMHO using some other names might just add to the confusion.

> 
>> +
>> +Example (Nokia N9):
>> +
>> +	als_ps@39 {
>> +		compatible = "avago,apds990x";
>> +		reg = <0x39>;
>> +
>> +		interrupt-parent = <&gpio3>;
>> +		interrupts = <19 10>; /* gpio_83, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_LOW */
>> +
>> +		Vdd-supply = <&vaux1>;
>> +		Vled-supply = <&vbat>;
>> +
>> +		ga	= <168834>;
>> +		cf1	= <4096>;
>> +		irf1	= <7824>;
>> +		cf2	= <877>;
>> +		irf2	= <1575>;
>> +		df	= <52>;
>> +
>> +		pdrive	= <0x2>; /* APDS_IRLED_CURR_25mA */
>> +		ppcount	= <5>;
>> +	};
>>
> 

Best regards,
Filip
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Device tree binding for Avago APDS990X light sensor

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

Hi!

> > +Required properties:
> > +- compatible: "avago,apds990x"
> > +- reg: address on the I2C bus
> > +- interrupts: external interrupt line number
> > +- Vdd-supply: power supply for VDD
> > +- Vled-supply: power supply for LEDA
> 
> AFAIK the custom is to use lower case letters for regulator supplies.
> 
> > +- ga: Glass attenuation
> > +- cf1: Clear channel factor 1
> > +- irf1: IR channel factor 1
> > +- cf2: Clear channel factor 2
> > +- irf2: IR channel factor 2
> > +- df: Device factor
> > +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
> > +- ppcount: Proximity pulse count
> 
> Are these device specific? If so, please add the vendor prefix to them.

Well, whole binding is "vendor specific". Does it make sense to add
prefix in such case?

> I might not use short abbreviations such as "df" either. I wonder what
> others think.

I see.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] Device tree binding for Avago APDS990X light sensor

[Sakari Ailus]

On Wed, Dec 27, 2017 at 07:50:42PM +0100, Filip Matijević wrote:
> Hi Sakari,
> 
> and thank you for your input - I've added a few comments below.
> 
> On 12/27/2017 07:00 PM, Sakari Ailus wrote:
> > Hi Pavel,
> > 
> > Thanks for the patch. Please see my comments below.
> > 
> > On Wed, Dec 27, 2017 at 10:18:28AM +0100, Pavel Machek wrote:
> >> From: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>
> >> This prepares binding for light sensor used in Nokia N9. 
> >>
> >> Signed-off-by: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> Signed-off-by: Pavel machek <pavel-+ZI9xUNit7I@public.gmane.org>
> >>
> >> ---
> >>
> >> Patches to convert APDS990X driver to device tree and to switch to iio
> >> are available.
> >>
> >> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> >> new file mode 100644
> >> index 0000000..e038146
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> >> @@ -0,0 +1,39 @@
> >> +Avago APDS990X driver
> >> +
> >> +Required properties:
> >> +- compatible: "avago,apds990x"
> >> +- reg: address on the I2C bus
> >> +- interrupts: external interrupt line number
> >> +- Vdd-supply: power supply for VDD
> >> +- Vled-supply: power supply for LEDA
> > 
> > AFAIK the custom is to use lower case letters for regulator supplies.
> 
> I've just used the same notation as in current driver. Datasheet calls
> those VDD (with DD being in subscript) and LEDA. I see no problem in
> changing those to vdd-supply and vled-supply if no one else objects.
> 
> > 
> >> +- ga: Glass attenuation
> >> +- cf1: Clear channel factor 1
> >> +- irf1: IR channel factor 1
> >> +- cf2: Clear channel factor 2
> >> +- irf2: IR channel factor 2
> >> +- df: Device factor
> >> +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
> >> +- ppcount: Proximity pulse count
> > 
> > Are these device specific? If so, please add the vendor prefix to them.
> 
> AFAIK yes - same as before if no one else objects, these will be changed.
> 
> > 
> > I might not use short abbreviations such as "df" either. I wonder what
> > others think.
> 
> These are also come from current driver - some of these comes from the
> datasheet itself, but not all. For example "ga" and "df" are in there
> (so I I would leave those alone), but "irf1" is called "B", "cf2" is
> called "C" and "irf2" is called "D" (I guess the missing "cf1" should be
> "A", but there is no such thing in the datasheet).
> IMHO using some other names might just add to the confusion.

Ack, datasheet names are fine.

You could also use a single property with all device specific coefficients
in a pre-defined order.

I don't have a strong opinion either way.

-- 
Regards,

Sakari Ailus
sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Device tree binding for Avago APDS990X light sensor

[Sakari Ailus]

On Wed, Dec 27, 2017 at 09:01:47PM +0100, Pavel Machek wrote:
> Hi!
> 
> > > +Required properties:
> > > +- compatible: "avago,apds990x"
> > > +- reg: address on the I2C bus
> > > +- interrupts: external interrupt line number
> > > +- Vdd-supply: power supply for VDD
> > > +- Vled-supply: power supply for LEDA
> > 
> > AFAIK the custom is to use lower case letters for regulator supplies.
> > 
> > > +- ga: Glass attenuation
> > > +- cf1: Clear channel factor 1
> > > +- irf1: IR channel factor 1
> > > +- cf2: Clear channel factor 2
> > > +- irf2: IR channel factor 2
> > > +- df: Device factor
> > > +- pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
> > > +- ppcount: Proximity pulse count
> > 
> > Are these device specific? If so, please add the vendor prefix to them.
> 
> Well, whole binding is "vendor specific". Does it make sense to add
> prefix in such case?

Yes, it does. If you later find one or more of these are generic, you could
remove the vendor prefix. I doubt that'll happen though, these seem very
device specific parameters.

-- 
Sakari Ailus
sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] Device tree binding for Avago APDS990X light sensor

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1749 bytes --]

From: Filip Matijević <filip.matijevic.pz@gmail.com>

This prepares binding for light sensor used in Nokia N9.

Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
new file mode 100644
index 0000000..480c0b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
@@ -0,0 +1,41 @@
+Avago APDS990X driver
+
+https://docs.broadcom.com/docs/AV02-2867EN
+
+Required properties:
+- compatible: "avago,apds990x"
+- reg: address on the I2C bus
+- interrupts: external interrupt line number
+- vdd-supply: power supply for VDD
+- vled-supply: power supply for LEDA
+- avago,ga: Glass attenuation
+- avago,cf1: Clear channel factor 1
+- avago,irf1: IR channel factor 1
+- avago,cf2: Clear channel factor 2
+- avago,irf2: IR channel factor 2
+- avago,df: Device factor
+- avago,pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
+- avago,ppcount: Proximity pulse count
+
+Example (Nokia N9):
+
+	als_ps@39 {
+		compatible = "avago,apds990x";
+		reg = <0x39>;
+
+		interrupt-parent = <&gpio3>;
+		interrupts = <19 (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_LEVEL_LOW)>; /* gpio_83 */
+
+		vdd-supply = <&vaux1>;
+		vled-supply = <&vbat>;
+
+		avago,ga	= <168834>;
+		avago,cf1	= <4096>;
+		avago,irf1	= <7824>;
+		avago,cf2	= <877>;
+		avago,irf2	= <1575>;
+		avago,df	= <52>;
+
+		avago,pdrive	= <0x2>; /* APDS_IRLED_CURR_25mA */
+		avago,ppcount	= <5>;
+	};

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCHv2] Device tree binding for Avago APDS990X light sensor

[Rob Herring]

On Tue, Jan 02, 2018 at 01:44:51PM +0100, Pavel Machek wrote:
> From: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> This prepares binding for light sensor used in Nokia N9.

"dt-bindings: ..." is the preferred subject prefix.

> 
> Signed-off-by: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> 
> diff --git a/Documentation/devicetree/bindings/misc/avago-apds990x.txt b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> new file mode 100644
> index 0000000..480c0b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt

Put this with other light sensors whether you use IIO or not:

bindings/iio/light/

> @@ -0,0 +1,41 @@
> +Avago APDS990X driver

Bindings aren't drivers.

> +
> +https://docs.broadcom.com/docs/AV02-2867EN
> +
> +Required properties:
> +- compatible: "avago,apds990x"
> +- reg: address on the I2C bus
> +- interrupts: external interrupt line number
> +- vdd-supply: power supply for VDD
> +- vled-supply: power supply for LEDA
> +- avago,ga: Glass attenuation

We already have "upisemi,glass-coef". Can we align on something common.

> +- avago,cf1: Clear channel factor 1
> +- avago,irf1: IR channel factor 1
> +- avago,cf2: Clear channel factor 2
> +- avago,irf2: IR channel factor 2

Perhaps 2 properties with 2 cells for factor 1 and 2.

> +- avago,df: Device factor

Units/range for all these?

> +- avago,pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values

Don't we have standard current property for LEDs?

> +- avago,ppcount: Proximity pulse count

Is this standard for prox sensors?

> +
> +Example (Nokia N9):
> +
> +	als_ps@39 {

light-sensor@39

> +		compatible = "avago,apds990x";
> +		reg = <0x39>;
> +
> +		interrupt-parent = <&gpio3>;
> +		interrupts = <19 (IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_LEVEL_LOW)>; /* gpio_83 */
> +
> +		vdd-supply = <&vaux1>;
> +		vled-supply = <&vbat>;
> +
> +		avago,ga	= <168834>;
> +		avago,cf1	= <4096>;
> +		avago,irf1	= <7824>;
> +		avago,cf2	= <877>;
> +		avago,irf2	= <1575>;
> +		avago,df	= <52>;
> +
> +		avago,pdrive	= <0x2>; /* APDS_IRLED_CURR_25mA */
> +		avago,ppcount	= <5>;
> +	};
> 
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3] dt-bindings: Avago APDS990X light sensor

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]

Hi!

> > From: Filip Matijević <filip.matijevic.pz@gmail.com>
> > 
> > This prepares binding for light sensor used in Nokia N9.
> 
> "dt-bindings: ..." is the preferred subject prefix.

Ok.

> > +++ b/Documentation/devicetree/bindings/misc/avago-apds990x.txt
> 
> Put this with other light sensors whether you use IIO or not:
> 
> bindings/iio/light/

Makes sense. Patches to convert to IIO are available.

> > @@ -0,0 +1,41 @@
> > +Avago APDS990X driver
> 
> Bindings aren't drivers.

Ok.

> > +https://docs.broadcom.com/docs/AV02-2867EN
> > +
> > +Required properties:
> > +- compatible: "avago,apds990x"
> > +- reg: address on the I2C bus
> > +- interrupts: external interrupt line number
> > +- vdd-supply: power supply for VDD
> > +- vled-supply: power supply for LEDA
> > +- avago,ga: Glass attenuation
> 
> We already have "upisemi,glass-coef". Can we align on something
> common.

upisemi seems to have different units (1/1000 vs. 1/4096). Do you want
me to do change to avago,glass-coef or ....?

> > +- avago,cf1: Clear channel factor 1
> > +- avago,irf1: IR channel factor 1
> > +- avago,cf2: Clear channel factor 2
> > +- avago,irf2: IR channel factor 2
> 
> Perhaps 2 properties with 2 cells for factor 1 and 2.

This is obviously easier to keep, but I can change.

> > +- avago,df: Device factor
> 
> Units/range for all these?

Fixed point, with 4096 == 1.00.

> > +- avago,pdrive: IR current, one of APDS_IRLED_CURR_XXXmA values
> 
> Don't we have standard current property for LEDs?

We do have "led-max-microamp"; which is not quite what this is. I
guess we can do "led-microamp" if you prefer?

> > +- avago,ppcount: Proximity pulse count
> 
> Is this standard for prox sensors?

I don't know many proximity sensors. Seems like way to raise sensitivity.

Best regards,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[PATCH] dt-bindings: Nokia N9 audio support

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 4245 bytes --]

From: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Add bindings for Nokia N9 audio components.

Signed-off-by: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>

diff --git a/Documentation/devicetree/bindings/media/ti-wl1273.txt b/Documentation/devicetree/bindings/media/ti-wl1273.txt
new file mode 100644
index 0000000..21db389
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti-wl1273.txt
@@ -0,0 +1,36 @@
+Texas Instruments - wl1273 radio/bluetooth module
+
+Required properties:
+
+- compatible - "ti,wl1273-core"
+- reg - I2C slave address
+- interrupts       - The interrupt output from the device.
+- interrupt-parent - The parent interrupt controller.
+- power-gpio       - gpio pin to power the device.
+
+- wl1273radio child - compatible = "ti,wl1273-fm-radio";
+
+Optional properties:
+
+- wl1273codec child - compatible = "ti,wl1273codec";
+
+Example:
+
+wl1273core: wl1273core@22 {
+		compatible = "ti,wl1273-core";
+		reg = <0x22>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&wl1273_gpio>;
+
+		interrupt-parent = <&gpio2>;
+		interrupts = <11 2>; /* gpio_43, IRQF_TRIGGER_FALLING */
+
+		wl1273radio: wl1273radio {
+			compatible = "ti,wl1273-fm-radio";
+		};
+
+		wl1273codec: wl1273codec {
+			compatible = "ti,wl1273-codec";
+		};
+	};
diff --git a/Documentation/devicetree/bindings/sound/nokia,n9.txt b/Documentation/devicetree/bindings/sound/nokia,n9.txt
new file mode 100644
index 0000000..230b1eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/nokia,n9.txt
@@ -0,0 +1,32 @@
+* Nokia N9/N950 audio setup
+
+Required properties:
+- compatible: "nokia,n9-audio"
+
+- nokia,twl4030-cpu-dai: phandle for the McBSP node connected to TWL4030
+- nokia,tlv320dac33-cpu-dai: phandle for the McBSP node connected to TLV320DAC33
+- nokia,wl1273-cpu-dai: phandle for the McBSP node connected to WL1273
+
+- nokia,twl4030-codec: phandle for the TWL4030 codec
+- nokia,tlv320dac33-codec: phandle for the TLV320DAC33 codec
+- nokia,wl1273-codec: phandle for the WL1273 codec
+- nokia,headphone-amplifier: phandle for the TPA6130A2 node
+
+- nokia,speaker-amplifier-gpios: GPIO for speaker amplifier
+
+Example:
+
+sound: n9-audio {
+	compatible = "nokia,n9-audio";
+
+	nokia,twl4030-cpu-dai = <&mcbsp3>;
+	nokia,tlv320dac33-cpu-dai = <&mcbsp2>;
+	nokia,wl1273-cpu-dai = <&mcbsp4>;
+
+	nokia,twl4030-codec = <&twl4030_audio>;
+	nokia,tlv320dac33-codec = <&tlv320dac33>;
+	nokia,wl1273-codec = <&wl1273codec>;
+	nokia,headphone-amplifier = <&tpa6140a2>;
+
+	nokia,speaker-amplifier-gpios = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
+};
diff --git a/Documentation/devicetree/bindings/sound/tlv320dac33.txt b/Documentation/devicetree/bindings/sound/tlv320dac33.txt
new file mode 100644
index 0000000..8e981f7
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tlv320dac33.txt
@@ -0,0 +1,39 @@
+Texas Instruments - tlv320dac33 Codec module
+
+The tlv320dac33 serial control bus communicates through I2C protocols.
+
+Required properties:
+
+- compatible - "ti,tlv320dac33"
+- reg - I2C slave address
+
+Optional properties:
+
+- power-gpio - gpio pin to power the device
+
+- avdd-supply, dvdd-supply, iovdd-supply: power supplies for the device as covered
+  in Documentation/devicetree/bindings/regulator/regulator.txt
+
+- interrupts       - The interrupt output from the device.
+- interrupt-parent - The parent interrupt controller.
+
+- ti,keep-bclk 	- Keep the BCLK running in FIFO modes
+- ti,burst-bclkdiv - BCLK divider value in burst mode
+
+Example:
+
+tlv320dac33: tlv320dac33@19 {
+	compatible = "ti,tlv320dac33";
+	reg = <0x19>;
+
+	avdd-supply = <&vaux4>;
+	dvdd-supply = <&vio>;
+	iovdd-supply = <&vio>;
+
+	interrupt-parent = <&gpio2>;
+	interrupts = <21 1>; /* gpio_53, IRQF_TRIGGER_RISING */
+	power-gpio = <&gpio2 28 0>; /* gpio_60 */
+
+	ti,keep-bclk;
+	ti,burst-bclkdiv = /bits/ 8 <3>;
+};


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] dt-bindings: Nokia N9 audio support

[Sakari Ailus]

Hi Pavel,

On Wed, Jan 10, 2018 at 09:53:15AM +0100, Pavel Machek wrote:
> From: Filip Matijević <filip.matijevic.pz@gmail.com>
> 
> Add bindings for Nokia N9 audio components.
> 
> Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/Documentation/devicetree/bindings/media/ti-wl1273.txt b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> new file mode 100644
> index 0000000..21db389
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> @@ -0,0 +1,36 @@
> +Texas Instruments - wl1273 radio/bluetooth module
> +
> +Required properties:
> +
> +- compatible - "ti,wl1273-core"
> +- reg - I2C slave address
> +- interrupts       - The interrupt output from the device.
> +- interrupt-parent - The parent interrupt controller.
> +- power-gpio       - gpio pin to power the device.
> +
> +- wl1273radio child - compatible = "ti,wl1273-fm-radio";

I'd document this under a separate section, as the compatible property for
the other child node.

You use "property - description" here but "property: description" in the
file below. It'd be nice to be consistent, albeit it's separate files.

> +
> +Optional properties:
> +
> +- wl1273codec child - compatible = "ti,wl1273codec";

"ti,wl1273-codec"

> +
> +Example:
> +
> +wl1273core: wl1273core@22 {
> +		compatible = "ti,wl1273-core";
> +		reg = <0x22>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wl1273_gpio>;
> +
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <11 2>; /* gpio_43, IRQF_TRIGGER_FALLING */
> +
> +		wl1273radio: wl1273radio {
> +			compatible = "ti,wl1273-fm-radio";
> +		};
> +
> +		wl1273codec: wl1273codec {
> +			compatible = "ti,wl1273-codec";
> +		};
> +	};
> diff --git a/Documentation/devicetree/bindings/sound/nokia,n9.txt b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> new file mode 100644
> index 0000000..230b1eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> @@ -0,0 +1,32 @@
> +* Nokia N9/N950 audio setup
> +

A small description saying what this really is would be nice.

> +Required properties:
> +- compatible: "nokia,n9-audio"
> +
> +- nokia,twl4030-cpu-dai: phandle for the McBSP node connected to TWL4030
> +- nokia,tlv320dac33-cpu-dai: phandle for the McBSP node connected to TLV320DAC33
> +- nokia,wl1273-cpu-dai: phandle for the McBSP node connected to WL1273
> +
> +- nokia,twl4030-codec: phandle for the TWL4030 codec
> +- nokia,tlv320dac33-codec: phandle for the TLV320DAC33 codec
> +- nokia,wl1273-codec: phandle for the WL1273 codec
> +- nokia,headphone-amplifier: phandle for the TPA6130A2 node
> +
> +- nokia,speaker-amplifier-gpios: GPIO for speaker amplifier
> +
> +Example:
> +
> +sound: n9-audio {
> +	compatible = "nokia,n9-audio";
> +
> +	nokia,twl4030-cpu-dai = <&mcbsp3>;
> +	nokia,tlv320dac33-cpu-dai = <&mcbsp2>;
> +	nokia,wl1273-cpu-dai = <&mcbsp4>;
> +
> +	nokia,twl4030-codec = <&twl4030_audio>;
> +	nokia,tlv320dac33-codec = <&tlv320dac33>;
> +	nokia,wl1273-codec = <&wl1273codec>;
> +	nokia,headphone-amplifier = <&tpa6140a2>;
> +
> +	nokia,speaker-amplifier-gpios = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
> +};
> diff --git a/Documentation/devicetree/bindings/sound/tlv320dac33.txt b/Documentation/devicetree/bindings/sound/tlv320dac33.txt
> new file mode 100644
> index 0000000..8e981f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/tlv320dac33.txt
> @@ -0,0 +1,39 @@
> +Texas Instruments - tlv320dac33 Codec module
> +
> +The tlv320dac33 serial control bus communicates through I2C protocols.
> +
> +Required properties:
> +
> +- compatible - "ti,tlv320dac33"
> +- reg - I2C slave address
> +
> +Optional properties:
> +
> +- power-gpio - gpio pin to power the device
> +
> +- avdd-supply, dvdd-supply, iovdd-supply: power supplies for the device as covered
> +  in Documentation/devicetree/bindings/regulator/regulator.txt
> +
> +- interrupts       - The interrupt output from the device.
> +- interrupt-parent - The parent interrupt controller.
> +
> +- ti,keep-bclk 	- Keep the BCLK running in FIFO modes
> +- ti,burst-bclkdiv - BCLK divider value in burst mode
> +
> +Example:
> +
> +tlv320dac33: tlv320dac33@19 {
> +	compatible = "ti,tlv320dac33";
> +	reg = <0x19>;
> +
> +	avdd-supply = <&vaux4>;
> +	dvdd-supply = <&vio>;
> +	iovdd-supply = <&vio>;
> +
> +	interrupt-parent = <&gpio2>;
> +	interrupts = <21 1>; /* gpio_53, IRQF_TRIGGER_RISING */
> +	power-gpio = <&gpio2 28 0>; /* gpio_60 */
> +
> +	ti,keep-bclk;
> +	ti,burst-bclkdiv = /bits/ 8 <3>;
> +};
> 

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH] dt-bindings: Nokia N9 audio support

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2237 bytes --]

Hi!

> On Wed, Jan 10, 2018 at 09:53:15AM +0100, Pavel Machek wrote:
> > From: Filip Matijević <filip.matijevic.pz@gmail.com>
> > 
> > Add bindings for Nokia N9 audio components.
> > 
> > Signed-off-by: Filip Matijević <filip.matijevic.pz@gmail.com>
> > Signed-off-by: Pavel Machek <pavel@ucw.cz>
> > 
> > diff --git a/Documentation/devicetree/bindings/media/ti-wl1273.txt b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> > new file mode 100644
> > index 0000000..21db389
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> > @@ -0,0 +1,36 @@
> > +Texas Instruments - wl1273 radio/bluetooth module
> > +
> > +Required properties:
> > +
> > +- compatible - "ti,wl1273-core"
> > +- reg - I2C slave address
> > +- interrupts       - The interrupt output from the device.
> > +- interrupt-parent - The parent interrupt controller.
> > +- power-gpio       - gpio pin to power the device.
> > +
> > +- wl1273radio child - compatible = "ti,wl1273-fm-radio";
> 
> I'd document this under a separate section, as the compatible property for
> the other child node.
> 
> You use "property - description" here but "property: description" in the
> file below. It'd be nice to be consistent, albeit it's separate
> files.

Heh. If we want to do consistency, it should be consistent in whole
documentation. I looked at cec-gpio.txt and it uses both
styles. Sampled few more files, and seems ":" is more common here.

> > +
> > +Optional properties:
> > +
> > +- wl1273codec child - compatible = "ti,wl1273codec";
> 
> "ti,wl1273-codec"

Ok.

> > diff --git a/Documentation/devicetree/bindings/sound/nokia,n9.txt b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> > new file mode 100644
> > index 0000000..230b1eb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> > @@ -0,0 +1,32 @@
> > +* Nokia N9/N950 audio setup
> > +
> 
> A small description saying what this really is would be nice.

I'm actually not sure what to say here. Suggestions?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH] dt-bindings: Nokia N9 audio support

[Rob Herring]

On Wed, Jan 10, 2018 at 09:53:15AM +0100, Pavel Machek wrote:
> From: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Add bindings for Nokia N9 audio components.
> 
> Signed-off-by: Filip Matijević <filip.matijevic.pz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> 
> diff --git a/Documentation/devicetree/bindings/media/ti-wl1273.txt b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> new file mode 100644
> index 0000000..21db389
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> @@ -0,0 +1,36 @@
> +Texas Instruments - wl1273 radio/bluetooth module

bluetooth chips have a binding location: bindings/net/bluetooth.

And we already have a WL1273 binding. Plus there's the one for the Nokia 
BT which I think can be TI chips with a different firmware and protocol. 
Is this related?

> +
> +Required properties:
> +
> +- compatible - "ti,wl1273-core"
> +- reg - I2C slave address
> +- interrupts       - The interrupt output from the device.
> +- interrupt-parent - The parent interrupt controller.
> +- power-gpio       - gpio pin to power the device.
> +
> +- wl1273radio child - compatible = "ti,wl1273-fm-radio";

AIUI, the FM radio is accessed thru the HCI interface aka shared 
transport via the UART. So it is already covered by the serial device 
binding for TI chips.

> +
> +Optional properties:
> +
> +- wl1273codec child - compatible = "ti,wl1273codec";

This is a codec or just BT audio I2S/PCM interface?

> +
> +Example:
> +
> +wl1273core: wl1273core@22 {
> +		compatible = "ti,wl1273-core";
> +		reg = <0x22>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wl1273_gpio>;
> +
> +		interrupt-parent = <&gpio2>;
> +		interrupts = <11 2>; /* gpio_43, IRQF_TRIGGER_FALLING */
> +
> +		wl1273radio: wl1273radio {
> +			compatible = "ti,wl1273-fm-radio";
> +		};
> +
> +		wl1273codec: wl1273codec {
> +			compatible = "ti,wl1273-codec";
> +		};
> +	};
> diff --git a/Documentation/devicetree/bindings/sound/nokia,n9.txt b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> new file mode 100644
> index 0000000..230b1eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> @@ -0,0 +1,32 @@
> +* Nokia N9/N950 audio setup
> +
> +Required properties:
> +- compatible: "nokia,n9-audio"
> +
> +- nokia,twl4030-cpu-dai: phandle for the McBSP node connected to TWL4030
> +- nokia,tlv320dac33-cpu-dai: phandle for the McBSP node connected to TLV320DAC33
> +- nokia,wl1273-cpu-dai: phandle for the McBSP node connected to WL1273
> +
> +- nokia,twl4030-codec: phandle for the TWL4030 codec
> +- nokia,tlv320dac33-codec: phandle for the TLV320DAC33 codec
> +- nokia,wl1273-codec: phandle for the WL1273 codec

Both dais and codecs should just be lists with a defined order.

> +- nokia,headphone-amplifier: phandle for the TPA6130A2 node

The simple card or graph card binding don't work? At least follow them 
to the extent that they do work. 
> +
> +- nokia,speaker-amplifier-gpios: GPIO for speaker amplifier
> +
> +Example:
> +
> +sound: n9-audio {
> +	compatible = "nokia,n9-audio";
> +
> +	nokia,twl4030-cpu-dai = <&mcbsp3>;
> +	nokia,tlv320dac33-cpu-dai = <&mcbsp2>;
> +	nokia,wl1273-cpu-dai = <&mcbsp4>;
> +
> +	nokia,twl4030-codec = <&twl4030_audio>;
> +	nokia,tlv320dac33-codec = <&tlv320dac33>;
> +	nokia,wl1273-codec = <&wl1273codec>;
> +	nokia,headphone-amplifier = <&tpa6140a2>;
> +
> +	nokia,speaker-amplifier-gpios = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
> +};
> diff --git a/Documentation/devicetree/bindings/sound/tlv320dac33.txt b/Documentation/devicetree/bindings/sound/tlv320dac33.txt
> new file mode 100644
> index 0000000..8e981f7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/tlv320dac33.txt
> @@ -0,0 +1,39 @@
> +Texas Instruments - tlv320dac33 Codec module
> +
> +The tlv320dac33 serial control bus communicates through I2C protocols.
> +
> +Required properties:
> +
> +- compatible - "ti,tlv320dac33"
> +- reg - I2C slave address
> +
> +Optional properties:
> +
> +- power-gpio - gpio pin to power the device

power-gpios.

Active high or low?

> +
> +- avdd-supply, dvdd-supply, iovdd-supply: power supplies for the device as covered
> +  in Documentation/devicetree/bindings/regulator/regulator.txt
> +
> +- interrupts       - The interrupt output from the device.
> +- interrupt-parent - The parent interrupt controller.
> +
> +- ti,keep-bclk 	- Keep the BCLK running in FIFO modes
> +- ti,burst-bclkdiv - BCLK divider value in burst mode
> +
> +Example:
> +
> +tlv320dac33: tlv320dac33@19 {

audio-codec@19

> +	compatible = "ti,tlv320dac33";
> +	reg = <0x19>;
> +
> +	avdd-supply = <&vaux4>;
> +	dvdd-supply = <&vio>;
> +	iovdd-supply = <&vio>;
> +
> +	interrupt-parent = <&gpio2>;
> +	interrupts = <21 1>; /* gpio_53, IRQF_TRIGGER_RISING */
> +	power-gpio = <&gpio2 28 0>; /* gpio_60 */
> +
> +	ti,keep-bclk;
> +	ti,burst-bclkdiv = /bits/ 8 <3>;
> +};
> 
> 
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dt-bindings: Nokia N9 audio support

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

Hi!

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti-wl1273.txt
> > @@ -0,0 +1,36 @@
> > +Texas Instruments - wl1273 radio/bluetooth module
> 
> bluetooth chips have a binding location: bindings/net/bluetooth.
> 
> And we already have a WL1273 binding. Plus there's the one for the Nokia 
> BT which I think can be TI chips with a different firmware and protocol. 
> Is this related?

Ok, let me investigate wl1273 (and postpone it, audio is critical, FM
radio -- not so much).

> > index 0000000..230b1eb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/nokia,n9.txt
> > @@ -0,0 +1,32 @@
> > +* Nokia N9/N950 audio setup
> > +
> > +Required properties:
> > +- compatible: "nokia,n9-audio"
> > +
> > +- nokia,twl4030-cpu-dai: phandle for the McBSP node connected to TWL4030
> > +- nokia,tlv320dac33-cpu-dai: phandle for the McBSP node connected to TLV320DAC33
> > +- nokia,wl1273-cpu-dai: phandle for the McBSP node connected to WL1273
> > +
> > +- nokia,twl4030-codec: phandle for the TWL4030 codec
> > +- nokia,tlv320dac33-codec: phandle for the TLV320DAC33 codec
> > +- nokia,wl1273-codec: phandle for the WL1273 codec
> 
> Both dais and codecs should just be lists with a defined order.
> 
> > +- nokia,headphone-amplifier: phandle for the TPA6130A2 node
> 
> The simple card or graph card binding don't work? At least follow them 
> to the extent that they do work. 

Yep, that quite big changes for the driver... Thanks for pointers. I'm
not sure what features simple card / graph card miss. 

> > +Texas Instruments - tlv320dac33 Codec module
> > +
> > +The tlv320dac33 serial control bus communicates through I2C protocols.
> > +
> > +Required properties:
> > +
> > +- compatible - "ti,tlv320dac33"
> > +- reg - I2C slave address
> > +
> > +Optional properties:
> > +
> > +- power-gpio - gpio pin to power the device
> 
> power-gpios.
> 
> Active high or low?

Ok, this one can be fixed easily, and I guess tlv320dac33 driver might
be useful for someone else, so I'll try to merge it first.

Thanks for review,

> > +	ti,burst-bclkdiv = /bits/ 8 <3>;

Oh and I'll make this plain int. No need to play with bits.

Best regards
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]