linux-kernel - Re: [PATCH 1/2] ASoC: dt-bindings: Add adi,ssm3515 amp schema

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <0216884B-A6F4-46B0-AEB9-AA16B3F24C8C@cutebit.org>
Date:   Tue, 9 May 2023 21:38:46 +0200
From:   Martin Povišer <povik+lin@...ebit.org>
To:     Conor Dooley <conor@...nel.org>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Nuno Sá <nuno.sa@...log.com>,
        asahi@...ts.linux.dev, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ASoC: dt-bindings: Add adi,ssm3515 amp schema

Hello Conor,

> On 9. 5. 2023, at 20:25, Conor Dooley <conor@...nel.org> wrote:
> 
> Hey Martin,
> 
> On Tue, May 09, 2023 at 06:38:27PM +0200, Martin Povišer wrote:
>> Add a DT schema for the SSM3515 amp by Analog Devices. It's a simple
>> mono amplifier with digital input.
>> 
>> Signed-off-by: Martin Povišer <povik+lin@...ebit.org>
>> ---
>> .../bindings/sound/adi,ssm3515.yaml           | 66 +++++++++++++++++++
>> 1 file changed, 66 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/adi,ssm3515.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/sound/adi,ssm3515.yaml b/Documentation/devicetree/bindings/sound/adi,ssm3515.yaml
>> new file mode 100644
>> index 000000000000..19b7185ae8e2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/adi,ssm3515.yaml
>> @@ -0,0 +1,66 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2019-20 Texas Instruments Incorporated
> 
> Copyright here looks a little odd, copy & paste from the TI bindings you
> send patches for earlier, or intended?

Yeah, that must have been a copypaste from the other schemas. Not intended
in any case. Thanks for the catch.

> 
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/sound/adi,ssm3515.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> 
> Drop the quotes on these please.

OK

>> +
>> +  adi,ana-gain:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2, 3]
>> +    description: |
>> +      The value to be set in the ANA_GAIN register field on the codec. This determines
>> +      the full voltage span of the codec's analog output.
>> +
>> +      To quote the datasheet on the available options:
>> +
>> +        00: 8.4 V Full-Scale Gain Mapping
>> +        01: 12.6 V Full-Scale Gain Mapping
>> +        10: 14 V Full-Scale Gain Mapping
>> +        11: 15 V Full-Scale Gain Mapping
> 
> Putting register values into the DT does not seem correct, although
> I know nothing about sound and will have to defer to Krzysztof & Co. on
> that front.

It’s an analog gain selection, it just seemed most honest and convenient
to defer to the register field in describing of what this property does.

I thought this should be here, and not with the other gain controls to be
exported to userspace, since it relates to the supply voltage. You probably
can’t select a gain value close or over the amp’s supply and expect proper
functioning, so the value here should be the system’s integrator choice.

On second thought there doesn’t seem to be any risk of damage to the amp
itself, so the knowledge of what’s the optimal value could be left up to
ALSA use-case configuration, and there’s no real downside in treating it
like other gain controls.

I think I will remove it in the next iteration.

> Cheers,
> Conor.

Cheers, Martin