Re: [PATCH 2/4] ASoC: dt-bindings: Add support for the GPIOs driven amplifier

[Rob Herring]

On Wed, Apr 8, 2026 at 12:09 PM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
>
> Hi Rob, Mark,
>
> On Wed, 8 Apr 2026 07:29:01 -0500
> Rob Herring <robh@xxxxxxxxxx> wrote:
>
> ...
>
> > > +properties:
> > > +  compatible:
> > > +    const: audio-gpio-amp
> >
> > To be consistent with other GPIO controlled devices: gpio-audio-amp
>
> Ok.
>
> Mark suggested to merge this gpio-audio-amp with simple-amplifier.

Merging driver and merging binding are separate questions.

> This leads to the following question:
>
> Should I keep the 'gpio-audio-amp' compatible string ?

Yes. 'cause this binding isn't simple...

And I don't care for any compatible with 'simple' (or generic) in it.
Not sure why I agreed to that one. But I'm all for generic/common
drivers.

> Should I keep two bindings (this one and the simple-audio-amplifier.yaml) or
> should I merge bindings?

2 bindings. There's no overlap in property names.


> ...
> > > +  gain-gpios:
> > > +    description: |
> > > +      GPIOs to control the amplifier gain
> > > +
> > > +      The gain value is computed from GPIOs value from 0 to 2^N-1 with N the
> > > +      number of GPIO described. The first GPIO described is the lsb of the gain
> > > +      value.
> > > +
> > > +      For instance assuming 2 gpios
> > > +         gain-gpios = <&gpio1 GPIO_ACTIVE_HIGH> <&gpio2 GPIO_ACTIVE_HIGH>;
> > > +      The gain value will be the following:
> > > +
> > > +          gpio1 | gpio2 | gain
> > > +          ------+-------+-----
> > > +            0   |    0  | 0b00 -> 0
> > > +            1   |    0  | 0b01 -> 1
> > > +            0   |    1  | 0b10 -> 2
> > > +            1   |    1  | 0b11 -> 3
> > > +          ------+-------+-----
> > > +
> > > +      Note: The gain value, bits set to 1 or 0, indicate the state active (bit
> > > +            set) or the state inactive (bit unset) of the related GPIO. The
> > > +            physical voltage corresponding to this active/inactive state is
> > > +            given by the GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags.
> > > +
> > > +    minItems: 1
> > > +    maxItems: 32
> >
> > 2^32 levels? Seems like a bit much. Also, unless you can change the
> > values of all the GPIOs atomically, aren't you going to get some
> > artifacts while the gain is being changed? Unless you mute I guess.
>
> I didn't want to set a particular limit related to the number of GPIOs
> used for thje gain value. Of course 2^32 is obviously a lot.
>
> What do you think about 16 for maxItems?

What is the most you are aware of? Take that and double it.

Seems to me 256 levels would be way more than a human ear could distinguish.

> Related to Artifacts, yes they can probably be there. Also the mute feature
> is not required. Some hardware use only one GPIO and doesn't implement mute
> feature. In that case no artifacts can be present.
>
> If mute is implemented, it is the application responsibility to handle
> mute / unmute while changing the gain value. I don't think we can do anything
> at driver level to avoid those artifacts if any.
>
> >
> > > +
> > > +  gain-points:
> > > +    $ref: /schemas/types.yaml#/definitions/int32-matrix
> > > +    items:
> > > +      items:
> > > +        - description: The GPIOs value
> >
> > Can't this just be the index?
>
> Some GPIOs value can be skipped if they don't make any sense in the hardware
> design. With the index, this is not possible.
>
> gpios:
>   0b00 -3dB
>   0b01 0dB
>   0b10 Reserved, should not be used
>   0b11 +3dB
>
> With just the index, the reserved 0b10 value cannot be skipped. I would like
> to handle this kind of cases.

Okay.

> > If not, then gain-range could be expressed using gain-points instead.
>
> Do you have in mind something like the following?
>   gain-range = <0 (-300)>, <3 600>;
>
> defining the range from -3dB to +6dB with GPIOs value 0 for -3dB and 3 for +6dB.

Yes, but since you can have reserved values, that won't work.

> > > +        - description: The related amplifier gain in 0.01 dB unit
> > > +    minItems: 2
> > > +    description: |
> > > +      List of the GPIOs value / Gain value in dB pair defining the gain
> > > +      set on each GPIOs value.
> > > +
> > > +      With 2 GPIOs controlling the gain, GPIOs value can be 0, 1, 2 and 3.
> > > +      Assuming that GPIOs values set the hardware gains according to the
> > > +      following table:
> > > +
> > > +         GPIOs | Hardware
> > > +         value | amplification
> > > +         ------+--------------
> > > +           0   | -10.0 dB
> > > +           1   | +3.0 dB
> > > +           2   | 0 dB
> > > +           3   | +6.0 dB
> > > +         ------+--------------
> > > +
> > > +      The description using gain points can be:
> > > +        gain-points = <0 (-1000)>, <1 300>, <2 0>, <3 600>;
> > > +
> > > +  gain-range:
> > > +    $ref: /schemas/types.yaml#/definitions/int32-array
> > > +    items:
> > > +      - description: Gain in 0.01 dB unit when all GPIOs are inactive
> > > +      - description: Gain in 0.01 dB unit when all GPIOs are active
> > > +    description: |
> > > +      Gains (in 0.01 dB unit) set by the extremum (minimal and maximum) value
> > > +      of GPIOs. The following formula must be satisfied.
> > > +
> > > +               gain-range[1] - gain-range[0]
> > > +      Gain  = ------------------------------- x GPIO_value + gain-range[0]
> > > +                        2^N - 1
> > > +
> > > +      With N, the number of GPIOs used to control the gain and Gain computed in
> > > +      0.01 dB unit.
> > > +
> > > +      With 2 GPIOs controlling the gain, GPIOs value can be 0, 1, 2 and 3.
> > > +      Assuming that gain value set the hardware according to the following
> > > +      table:
> > > +
> > > +         GPIOs | Hardware 1    | Hardware 2
> > > +         value | amplification | amplification
> > > +         ------+---------------+---------------
> > > +           0   | -3.0 dB       |  +10.0 dB
> > > +           1   | 0 dB          |  +5.0 dB
> > > +           2   | +3.0 dB       |  0 dB
> > > +           3   | +6.0 dB       |  -5.0 dB
> > > +         ------+---------------+---------------
> > > +
> > > +      The description for hardware 1 using a gain range can be:
> > > +        gain-range = <(-300) 600>;
> > > +
> > > +      The description for hardware 2 using a gain range can be:
> > > +        gain-range = <1000 (-500)>;
> > > +
> > > +  gain-labels:
> > > +    $ref: /schemas/types.yaml#/definitions/string-array
> >
> > minItems: 2
> > maxItems: 0x100000000
>
> Ok, I will adjust maxItems according to the max number of GPIO supported.
>
> For my curiosity, is there a way to express maxItems with a computation
> based on some other properties value ?

No, there isn't.

>
> What could be relevant here is
>   maxitems: 2^(number of items available in the gpio-gain properties)
>
> >
> > > +    description: |
> > > +      List of the gain labels attached to the combination of GPIOs controlling
> > > +      the gain. The first label is related to the gain value 0, the second label
> > > +      is related to the gain value 1 and so on.
> > > +
> > > +      With 2 GPIOs controlling the gain, GPIOs value can be 0, 1, 2 and 3.
> > > +      Assuming that gain value set the hardware according to the following
> > > +      table:
> > > +
> > > +         GPIOs | Hardware
> > > +         value | amplification
> > > +         ------+--------------
> > > +           0   | Low
> > > +           1   | Middle
> > > +           2   | High
> > > +           3   | Max
> > > +         ------+--------------
> > > +
> > > +      The description using gain labels can be:
> > > +        gain-labels = "Low", "Middle", "High", "Max";
> >
> > Do we need to allow these to be anything? It's going to get hard to come
> > up with 2^32 names.
>
> Well, "Normal" / "Boost" can make sense on some hardware.
>
> I don't think we need to restrict labels to a list of known label here.

As long as the names are meaningless to software.

>
> Of course 2^32 names is obviously a lot. What could be the limit?

I would guess at 8 or more, it's just going to be gain1, gain2, etc.
or something similar constructed from the gain values.

> ...
>
> > > +
> > > +    /* A mutable amplifier without any gain control */
> > > +    amplifier4 {
> > > +        compatible = "audio-gpio-amp";
> > > +        vdd-supply = <&regulator>;
> > > +        mute-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
> >
> > This case is just simple-amplifier...
>
> No, simple-amplifier uses 'enable' and not 'mute'.

Yes, I know...

> We can have the amplifier enabled ('enable' GPIO active) as it is
> used and a switch driven by an other GPIO to mute / un-mute the
> amplifier output.

But you have no 'enable' GPIO here. To me, enable just looks like
inverted mute. If there's some electrical difference, I can't tell
what that is from either binding.

I guess my point was that really we could deprecate simple-amplifier
binding because this one can handle it and more. But I'm not
suggesting we do that yet.

Rob