diff mbox series

[v2,1/3] dt-bindings: ASoC: Add TDA7802 amplifier

Message ID 20190730120937.16271-2-thomas.preston@codethink.co.uk (mailing list archive)
State New, archived
Headers show
Series ASoC: Codecs: Add TDA7802 codec | expand

Commit Message

Thomas Preston July 30, 2019, 12:09 p.m. UTC
Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
Cc: Patrick Glaser <pglaser@tesla.com>
Cc: Rob Duncan <rduncan@tesla.com>
Cc: Nate Case <ncase@tesla.com>
---
 .../devicetree/bindings/sound/tda7802.txt     | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/tda7802.txt

Comments

Marco Felsch July 30, 2019, 1:12 p.m. UTC | #1
Hi Charles,

sorry for jumping in..

On 19-07-30 13:27, Charles Keepax wrote:
> On Tue, Jul 30, 2019 at 01:09:35PM +0100, Thomas Preston wrote:
> > Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> > Cc: Patrick Glaser <pglaser@tesla.com>
> > Cc: Rob Duncan <rduncan@tesla.com>
> > Cc: Nate Case <ncase@tesla.com>
> > ---
> >  .../devicetree/bindings/sound/tda7802.txt     | 26 +++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/sound/tda7802.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/tda7802.txt b/Documentation/devicetree/bindings/sound/tda7802.txt
> > new file mode 100644
> > index 000000000000..f80aaf4f1ba0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/tda7802.txt
> > @@ -0,0 +1,26 @@
> > +ST TDA7802 audio processor
> > +
> > +This device supports I2C only.
> > +
> > +Required properties:
> > +
> > +- compatible : "st,tda7802"
> > +- reg : the I2C address of the device
> > +- enable-supply : a regulator spec for the PLLen pin

Shouldn't that be a pin called 'pllen-gpios'? IMHO I would not use a
regulator for that.

Regards,
  Marco

> > +
> > +Optional properties:
> > +
> > +- st,gain-ch13 : gain for channels 1 and 3 (range: 1-4)
> > +- st,gain-ch24 : gain for channels 2 and 3 (range: 1-4)
> 
> I wouldn't have expected the gains to be available as a device
> tree setting.
> 
> > +- st,diagnostic-mode-ch13 : diagnotic mode for channels 1 and 3
> > +                            values: "Speaker" (default), "Booster"
> > +- st,diagnostic-mode-ch24 : diagnotic mode for channels 2 and 4
> > +                            values: "Speaker" (default), "Booster"
> > +
> > +Example:
> > +
> > +amp: tda7802@6c {
> > +	compatible = "st,tda7802";
> > +	reg = <0x6c>;
> > +	enable-supply = <&amp_enable_reg>;
> > +};
> > -- 
> > 2.21.0
> > 
> 
> Thanks,
> Charles
>
Thomas Preston July 30, 2019, 2:10 p.m. UTC | #2
On 30/07/2019 13:27, Charles Keepax wrote:
> On Tue, Jul 30, 2019 at 01:09:35PM +0100, Thomas Preston wrote:
>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
>> Cc: Patrick Glaser <pglaser@tesla.com>
>> Cc: Rob Duncan <rduncan@tesla.com>
>> Cc: Nate Case <ncase@tesla.com>
>> ---
>>  .../devicetree/bindings/sound/tda7802.txt     | 26 +++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/sound/tda7802.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/tda7802.txt b/Documentation/devicetree/bindings/sound/tda7802.txt
>> new file mode 100644
>> index 000000000000..f80aaf4f1ba0
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/tda7802.txt
>> @@ -0,0 +1,26 @@
>> +ST TDA7802 audio processor
>> +
>> +This device supports I2C only.
>> +
>> +Required properties:
>> +
>> +- compatible : "st,tda7802"
>> +- reg : the I2C address of the device
>> +- enable-supply : a regulator spec for the PLLen pin
>> +
>> +Optional properties:
>> +
>> +- st,gain-ch13 : gain for channels 1 and 3 (range: 1-4)
>> +- st,gain-ch24 : gain for channels 2 and 3 (range: 1-4)
> 
> I wouldn't have expected the gains to be available as a device
> tree setting.
> 

Ah, I forgot to update the docs from v1. Thanks
Thomas Preston July 30, 2019, 2:12 p.m. UTC | #3
On 30/07/2019 14:12, Marco Felsch wrote:
> Hi Charles,
> 
> sorry for jumping in..
> 
> On 19-07-30 13:27, Charles Keepax wrote:
>> On Tue, Jul 30, 2019 at 01:09:35PM +0100, Thomas Preston wrote:
>>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
>>> Cc: Patrick Glaser <pglaser@tesla.com>
>>> Cc: Rob Duncan <rduncan@tesla.com>
>>> Cc: Nate Case <ncase@tesla.com>
>>> ---
>>>  .../devicetree/bindings/sound/tda7802.txt     | 26 +++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/sound/tda7802.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/sound/tda7802.txt b/Documentation/devicetree/bindings/sound/tda7802.txt
>>> new file mode 100644
>>> index 000000000000..f80aaf4f1ba0
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/sound/tda7802.txt
>>> @@ -0,0 +1,26 @@
>>> +ST TDA7802 audio processor
>>> +
>>> +This device supports I2C only.
>>> +
>>> +Required properties:
>>> +
>>> +- compatible : "st,tda7802"
>>> +- reg : the I2C address of the device
>>> +- enable-supply : a regulator spec for the PLLen pin
> 
> Shouldn't that be a pin called 'pllen-gpios'? IMHO I would not use a
> regulator for that.
> 
> Regards,
>   Marco
> 

Hi Marco,
We have multiple amplifiers hooked up in a chain, and all the PLLens
are connected to one GPIO. So we need to use a regulator so that
i2c-TDA7802:00 doesn't turn off the PLLen which i2c-TDA7802:01 still
requires.

This is why we use a regulator. Is there GPIO support for this?

Thanks,
Thomas
Mark Brown July 30, 2019, 2:33 p.m. UTC | #4
On Tue, Jul 30, 2019 at 03:12:21PM +0100, Thomas Preston wrote:
> On 30/07/2019 14:12, Marco Felsch wrote:

> >>> +- compatible : "st,tda7802"
> >>> +- reg : the I2C address of the device
> >>> +- enable-supply : a regulator spec for the PLLen pin

> > Shouldn't that be a pin called 'pllen-gpios'? IMHO I would not use a
> > regulator for that.

> Hi Marco,
> We have multiple amplifiers hooked up in a chain, and all the PLLens
> are connected to one GPIO. So we need to use a regulator so that
> i2c-TDA7802:00 doesn't turn off the PLLen which i2c-TDA7802:01 still
> requires.

> This is why we use a regulator. Is there GPIO support for this?

If it's a GPIO not a regulator then it should be a GPIO not a regulator
in the device tree.  The device tree describes the hardware.  There was
some work on helping share GPIOs in the GPIO framework to accomodate
GPIOs for regulator enables, you should be able to do something similar
to what the regulator framework does.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/tda7802.txt b/Documentation/devicetree/bindings/sound/tda7802.txt
new file mode 100644
index 000000000000..f80aaf4f1ba0
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tda7802.txt
@@ -0,0 +1,26 @@ 
+ST TDA7802 audio processor
+
+This device supports I2C only.
+
+Required properties:
+
+- compatible : "st,tda7802"
+- reg : the I2C address of the device
+- enable-supply : a regulator spec for the PLLen pin
+
+Optional properties:
+
+- st,gain-ch13 : gain for channels 1 and 3 (range: 1-4)
+- st,gain-ch24 : gain for channels 2 and 3 (range: 1-4)
+- st,diagnostic-mode-ch13 : diagnotic mode for channels 1 and 3
+                            values: "Speaker" (default), "Booster"
+- st,diagnostic-mode-ch24 : diagnotic mode for channels 2 and 4
+                            values: "Speaker" (default), "Booster"
+
+Example:
+
+amp: tda7802@6c {
+	compatible = "st,tda7802";
+	reg = <0x6c>;
+	enable-supply = <&amp_enable_reg>;
+};