Device tree bindings for the ESS ES9218P codec, which uses an
I2C control interface.
Signed-off-by: Aidan MacDonald <[email protected]>
---
.../bindings/sound/ess,es9218p.yaml | 104 ++++++++++++++++++
1 file changed, 104 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/ess,es9218p.yaml
diff --git a/Documentation/devicetree/bindings/sound/ess,es9218p.yaml b/Documentation/devicetree/bindings/sound/ess,es9218p.yaml
new file mode 100644
index 000000000000..d205b6a8bbb2
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ess,es9218p.yaml
@@ -0,0 +1,104 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ess,es9218p.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ESS ES9218P audio codec
+
+maintainers:
+ - Aidan MacDonald <[email protected]>
+
+properties:
+ compatible:
+ const: ess,es9218p
+
+ reg:
+ maxItems: 1
+
+ "#sound-dai-cells":
+ const: 0
+
+ avdd-supply:
+ description:
+ Definition of the regulator used for digital core power supply.
+
+ vcca-supply:
+ description:
+ Definition of the regulator used for oscillator and DAC power supply.
+
+ avcc3v3-supply:
+ description:
+ Definition of the 3.3V regulator for amplifier, switch, and charge pumps.
+
+ avcc1v8-supply:
+ description:
+ Definition of the 1.8V regulator for amplifier, switch, and charge pumps.
+
+ clocks:
+ items:
+ - description: clock for master clock (MCLK)
+
+ clock-names:
+ items:
+ - const: mclk
+
+ reset-gpios:
+ maxItems: 1
+ description:
+ Pin used for codec hardware reset, corresponds to the RESETB pin.
+
+ ess,max-clock-div:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Sets the maximum MCLK divider for generating the internal CLK.
+ CLK must be at least 20x the I2C bus speed or I2C transactions
+ will fail. The maximum divider should be chosen to ensure that
+ CLK will not fall below the limit.
+ enum:
+ - 1
+ - 2
+ - 4
+ - 8
+ default: 1
+
+ ess,oscillator-pad-bias:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Sets the oscillator pad drive bias. 0 - full bias, 15 - disabled.
+ minimum: 0
+ maximum: 15
+ default: 0
+
+required:
+ - compatible
+ - reg
+ - "#sound-dai-cells"
+ - avdd-supply
+ - vcca-supply
+ - avcc3v3-supply
+ - avcc1v8-supply
+ - clocks
+ - clock-names
+ - reset-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ codec@48 {
+ compatible = "ess,es9218p";
+ reg = <0x48>;
+ #sound-dai-cells = <0>;
+ avdd-supply = <&avdd>;
+ vcca-supply = <&vcca>;
+ avcc3v3-supply = <&avcc3v3>;
+ avcc1v8-supply = <&avcc1v8>;
+ clocks = <&xtal_clock>;
+ clock-names = "mclk";
+ reset-gpios = <&gpio 1 GPIO_ACTIVE_LOW>;
+ };
+ };
--
2.39.2
On 10/05/2023 13:23, Aidan MacDonald wrote:
> Device tree bindings for the ESS ES9218P codec, which uses an
> I2C control interface.
>
Thank you for your patch. There is something to discuss/improve.
Subject: reverse dt-bindings and ASoC.
Subject: drop second/last, redundant "bindings". The "dt-bindings"
prefix is already stating that these are bindings.
> Signed-off-by: Aidan MacDonald <[email protected]>
> ---
> .../bindings/sound/ess,es9218p.yaml | 104 ++++++++++++++++++
> 1 file changed, 104 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/ess,es9218p.yaml
>
> diff --git a/Documentation/devicetree/bindings/sound/ess,es9218p.yaml b/Documentation/devicetree/bindings/sound/ess,es9218p.yaml
> new file mode 100644
> index 000000000000..d205b6a8bbb2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/ess,es9218p.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/ess,es9218p.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ESS ES9218P audio codec
> +
> +maintainers:
> + - Aidan MacDonald <[email protected]>
> +
allOf with ref to dai-common.
> +properties:
> + compatible:
> + const: ess,es9218p
> +
> + reg:
> + maxItems: 1
> +
> + "#sound-dai-cells":
> + const: 0
> +
> + avdd-supply:
> + description:
> + Definition of the regulator used for digital core power supply.
Drop "Definition of the regulator used for"
> +
> + vcca-supply:
> + description:
> + Definition of the regulator used for oscillator and DAC power supply.
> +
Ditto
> + avcc3v3-supply:
> + description:
> + Definition of the 3.3V regulator for amplifier, switch, and charge pumps.
Drop "Definition of"
> +
> + avcc1v8-supply:
> + description:
> + Definition of the 1.8V regulator for amplifier, switch, and charge pumps.
Ditto
> +
> + clocks:
> + items:
> + - description: clock for master clock (MCLK)
> +
> + clock-names:
> + items:
> + - const: mclk
Drop entire names, not really useful.
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + Pin used for codec hardware reset, corresponds to the RESETB pin.
> +
> + ess,max-clock-div:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Sets the maximum MCLK divider for generating the internal CLK.
> + CLK must be at least 20x the I2C bus speed or I2C transactions
> + will fail. The maximum divider should be chosen to ensure that
> + CLK will not fall below the limit.
> + enum:
> + - 1
> + - 2
> + - 4
> + - 8
> + default: 1
Why do you need to customize it per board?
> +
> + ess,oscillator-pad-bias:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: Sets the oscillator pad drive bias. 0 - full bias, 15 - disabled.
> + minimum: 0
> + maximum: 15
> + default: 0
> +
> +required:
> + - compatible
> + - reg
> + - "#sound-dai-cells"
> + - avdd-supply
> + - vcca-supply
> + - avcc3v3-supply
> + - avcc1v8-supply
> + - clocks
> + - clock-names
> + - reset-gpios
> +
> +additionalProperties: false
Instead: unevaluatedProperties: false
> +
> +examples:
Best regards,
Krzysztof
Krzysztof Kozlowski <[email protected]> writes:
> On 10/05/2023 13:23, Aidan MacDonald wrote:
>> ...
>> + ess,max-clock-div:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Sets the maximum MCLK divider for generating the internal CLK.
>> + CLK must be at least 20x the I2C bus speed or I2C transactions
>> + will fail. The maximum divider should be chosen to ensure that
>> + CLK will not fall below the limit.
>> + enum:
>> + - 1
>> + - 2
>> + - 4
>> + - 8
>> + default: 1
>
> Why do you need to customize it per board?
>
There's no generic API to read or change the bus speed (SCL frequency)
for I2C adapters, so it's impossible to calculate a limit on the MCLK
divider automatically.
Defaulting to 1 is always safe, but means wasting power at lower sample
rates. If you know what the bus speed is you can use a higher divider
limit to save power, and it has to be done at the board/firmware level
because that's the only place where the bus speed is known.
>> +
>> + ess,oscillator-pad-bias:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Sets the oscillator pad drive bias. 0 - full bias, 15 - disabled.
>> + minimum: 0
>> + maximum: 15
>> + default: 0
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - "#sound-dai-cells"
>> + - avdd-supply
>> + - vcca-supply
>> + - avcc3v3-supply
>> + - avcc1v8-supply
>> + - clocks
>> + - clock-names
>> + - reset-gpios
>> +
>> +additionalProperties: false
>
> Instead: unevaluatedProperties: false
>
>
>> +
>> +examples:
>
> Best regards,
> Krzysztof
Everything else agreed, I'll send v2 later today.
Regards,
Aidan
On 11/05/2023 12:15, Aidan MacDonald wrote:
>
> Krzysztof Kozlowski <[email protected]> writes:
>
>> On 10/05/2023 13:23, Aidan MacDonald wrote:
>>> ...
>>> + ess,max-clock-div:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description:
>>> + Sets the maximum MCLK divider for generating the internal CLK.
>>> + CLK must be at least 20x the I2C bus speed or I2C transactions
>>> + will fail. The maximum divider should be chosen to ensure that
>>> + CLK will not fall below the limit.
>>> + enum:
>>> + - 1
>>> + - 2
>>> + - 4
>>> + - 8
>>> + default: 1
>>
>> Why do you need to customize it per board?
>>
>
> There's no generic API to read or change the bus speed (SCL frequency)
> for I2C adapters, so it's impossible to calculate a limit on the MCLK
> divider automatically.
>
> Defaulting to 1 is always safe, but means wasting power at lower sample
> rates. If you know what the bus speed is you can use a higher divider
> limit to save power, and it has to be done at the board/firmware level
> because that's the only place where the bus speed is known.
If I understand correctly, you only miss a way to get bus_freq_hz from
i2c_timings to calculate the divider by yourself? This looks like Linux
limitation, so we shouldn't push it into DT, but rather fix Linux. The
I2C bus rate is known, the MCLK rate as well, so divider is possible to
deduce.
I am actually surprised that I2C core does not store the timings
anywhere and each bus host has to deal with it on its own.
Best regards,
Krzysztof
Krzysztof Kozlowski <[email protected]> writes:
> On 11/05/2023 12:15, Aidan MacDonald wrote:
>>
>> Krzysztof Kozlowski <[email protected]> writes:
>>
>>> On 10/05/2023 13:23, Aidan MacDonald wrote:
>>>> ...
>>>> + ess,max-clock-div:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + description:
>>>> + Sets the maximum MCLK divider for generating the internal CLK.
>>>> + CLK must be at least 20x the I2C bus speed or I2C transactions
>>>> + will fail. The maximum divider should be chosen to ensure that
>>>> + CLK will not fall below the limit.
>>>> + enum:
>>>> + - 1
>>>> + - 2
>>>> + - 4
>>>> + - 8
>>>> + default: 1
>>>
>>> Why do you need to customize it per board?
>>>
>>
>> There's no generic API to read or change the bus speed (SCL frequency)
>> for I2C adapters, so it's impossible to calculate a limit on the MCLK
>> divider automatically.
>>
>> Defaulting to 1 is always safe, but means wasting power at lower sample
>> rates. If you know what the bus speed is you can use a higher divider
>> limit to save power, and it has to be done at the board/firmware level
>> because that's the only place where the bus speed is known.
>
> If I understand correctly, you only miss a way to get bus_freq_hz from
> i2c_timings to calculate the divider by yourself? This looks like Linux
> limitation, so we shouldn't push it into DT, but rather fix Linux. The
> I2C bus rate is known, the MCLK rate as well, so divider is possible to
> deduce.
>
> I am actually surprised that I2C core does not store the timings
> anywhere and each bus host has to deal with it on its own.
>
> Best regards,
> Krzysztof
I agree it'd be better if Linux provided access the bus frequency, but
even if that API was added it will take time for every I2C adapter to
support it. So in that case we would still need a DT property to provide
a safe limit or use a safe default, and miss potential power savings.
I'd prefer to add the DT property to allow power savings to be had now,
and drop it if/when the kernel gets an API for bus frequency. That will
be safe from a compatibility point of view -- the property won't be
providing any useful information so it won't matter if old DTs have it.
Regards,
Aidan
On 11/05/2023 13:26, Aidan MacDonald wrote:
>
> Krzysztof Kozlowski <[email protected]> writes:
>
>> On 11/05/2023 12:15, Aidan MacDonald wrote:
>>>
>>> Krzysztof Kozlowski <[email protected]> writes:
>>>
>>>> On 10/05/2023 13:23, Aidan MacDonald wrote:
>>>>> ...
>>>>> + ess,max-clock-div:
>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>> + description:
>>>>> + Sets the maximum MCLK divider for generating the internal CLK.
>>>>> + CLK must be at least 20x the I2C bus speed or I2C transactions
>>>>> + will fail. The maximum divider should be chosen to ensure that
>>>>> + CLK will not fall below the limit.
>>>>> + enum:
>>>>> + - 1
>>>>> + - 2
>>>>> + - 4
>>>>> + - 8
>>>>> + default: 1
>>>>
>>>> Why do you need to customize it per board?
>>>>
>>>
>>> There's no generic API to read or change the bus speed (SCL frequency)
>>> for I2C adapters, so it's impossible to calculate a limit on the MCLK
>>> divider automatically.
>>>
>>> Defaulting to 1 is always safe, but means wasting power at lower sample
>>> rates. If you know what the bus speed is you can use a higher divider
>>> limit to save power, and it has to be done at the board/firmware level
>>> because that's the only place where the bus speed is known.
>>
>> If I understand correctly, you only miss a way to get bus_freq_hz from
>> i2c_timings to calculate the divider by yourself? This looks like Linux
>> limitation, so we shouldn't push it into DT, but rather fix Linux. The
>> I2C bus rate is known, the MCLK rate as well, so divider is possible to
>> deduce.
>>
>> I am actually surprised that I2C core does not store the timings
>> anywhere and each bus host has to deal with it on its own.
>>
>> Best regards,
>> Krzysztof
>
> I agree it'd be better if Linux provided access the bus frequency, but
> even if that API was added it will take time for every I2C adapter to
> support it. So in that case we would still need a DT property to provide
> a safe limit or use a safe default, and miss potential power savings.
>
> I'd prefer to add the DT property to allow power savings to be had now,
> and drop it if/when the kernel gets an API for bus frequency. That will
> be safe from a compatibility point of view -- the property won't be
> providing any useful information so it won't matter if old DTs have it.
OK, sounds good.
Best regards,
Krzysztof