diff mbox series

[v2,1/3] dt-bindings: sound: Add CS42L84 codec

Message ID 20241020-cs42l84-v2-1-37ba2b6721d9@gmail.com (mailing list archive)
State Accepted
Commit f2a67da9f4eb03f5402acb9aeb65b23cac990827
Headers show
Series ASoC: add CS42L84 codec driver | expand

Commit Message

James Calligeros Oct. 19, 2024, 2:47 p.m. UTC
From: Martin Povišer <povik+lin@cutebit.org>

CS42L84 is a headphone jack codec made by Cirrus Logic and seen in Apple
computer models starting with 2021 Macbook Pros. It is not a publicly
documented part. To a degree the part is similar to the public CS42L42.
(The L84 superseded L83 seen in earlier Apple models, and the L83 was
pretty much the same as L42.)

Signed-off-by: Martin Povišer <povik+lin@cutebit.org>
Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
Reviewed-by: Neal Gompa <neal@gompa.dev>
---
 .../bindings/sound/cirrus,cs42l84.yaml   | 56 +++++++++++++++++++++++++
 MAINTAINERS                              |  1 +
 2 files changed, 57 insertions(+)

Comments

Rob Herring (Arm) Oct. 21, 2024, 7:26 p.m. UTC | #1
On Sun, Oct 20, 2024 at 12:47:31AM +1000, James Calligeros wrote:
> From: Martin Povišer <povik+lin@cutebit.org>
> 
> CS42L84 is a headphone jack codec made by Cirrus Logic and seen in Apple
> computer models starting with 2021 Macbook Pros. It is not a publicly
> documented part. To a degree the part is similar to the public CS42L42.
> (The L84 superseded L83 seen in earlier Apple models, and the L83 was
> pretty much the same as L42.)

Why can't this be added to 
Documentation/devicetree/bindings/sound/cirrus,cs42l42.yaml?

I guess perhaps you don't know what the supplies look like? Do any of 
the custom properties apply?

> 
> Signed-off-by: Martin Povišer <povik+lin@cutebit.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> ---
>  .../bindings/sound/cirrus,cs42l84.yaml   | 56 +++++++++++++++++++++++++
>  MAINTAINERS                              |  1 +
>  2 files changed, 57 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..7f8338e8ae369bc529ac3cf35041d5a7b9f3e6d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/cirrus,cs42l84.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic CS42L84 audio CODEC
> +
> +maintainers:
> +  - Martin Povišer <povik+lin@cutebit.org>
> +
> +description: |

Don't need '|' if no formatting.

> +  The CS42L84 is a headphone jack codec made by Cirrus Logic and embedded
> +  in personal computers sold by Apple. It was first seen in 2021 Macbook
> +  Pro models. It has stereo DAC for playback, mono ADC for capture, and
> +  is somewhat similar to CS42L42 but with a different regmap.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - cirrus,cs42l84
> +
> +  reg:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  '#sound-dai-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      jack_codec: codec@4b {
> +          compatible = "cirrus,cs42l84";
> +          reg = <0x4b>;
> +          reset-gpios = <&pinctrl_nub 4 GPIO_ACTIVE_LOW>;
> +          interrupts-extended = <&pinctrl_ap 180 IRQ_TYPE_LEVEL_LOW>;
> +          #sound-dai-cells = <0>;
> +      };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c1a2c296446c0724a0c6e70c845e5e5e1e693fd5..f5f85714dc4e8ca9c60b3f6963b2cec1ea33fdd0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2132,6 +2132,7 @@ L:	asahi@lists.linux.dev
>  L:	linux-sound@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/sound/adi,ssm3515.yaml
> +F:	Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml
>  F:	Documentation/devicetree/bindings/sound/apple,*
>  F:	sound/soc/apple/*
>  F:	sound/soc/codecs/cs42l83-i2c.c
> 
> -- 
> 2.47.0
>
Mark Brown Oct. 21, 2024, 10:15 p.m. UTC | #2
On Mon, Oct 21, 2024 at 02:26:32PM -0500, Rob Herring wrote:
> On Sun, Oct 20, 2024 at 12:47:31AM +1000, James Calligeros wrote:

> > CS42L84 is a headphone jack codec made by Cirrus Logic and seen in Apple
> > computer models starting with 2021 Macbook Pros. It is not a publicly
> > documented part. To a degree the part is similar to the public CS42L42.
> > (The L84 superseded L83 seen in earlier Apple models, and the L83 was
> > pretty much the same as L42.)

> Why can't this be added to 
> Documentation/devicetree/bindings/sound/cirrus,cs42l42.yaml?

> I guess perhaps you don't know what the supplies look like? Do any of 
> the custom properties apply?

I don't know if the Cirrus people who are listed as maintainers of that
binding might have concerns about doing things that acknowledge this
particular part.
James Calligeros Oct. 22, 2024, 7:14 a.m. UTC | #3
Hi all,

On Mon, Oct 21, 2024 at 02:26:32PM -0500, Rob Herring wrote:
> On Sun, Oct 20, 2024 at 12:47:31AM +1000, James Calligeros wrote:
> > CS42L84 is a headphone jack codec made by Cirrus Logic and seen in Apple
> > computer models starting with 2021 Macbook Pros. It is not a publicly
> > documented part. To a degree the part is similar to the public CS42L42.
> > (The L84 superseded L83 seen in earlier Apple models, and the L83 was
> > pretty much the same as L42.)
> 
> Why can't this be added to
> Documentation/devicetree/bindings/sound/cirrus,cs42l42.yaml?
> 
> I guess perhaps you don't know what the supplies look like? Do any of
> the custom properties apply?

I suppose the original thinking around creating a new binding was simply new 
driver, new chip, new binding. We also don't describe the power supply of the 
chip at all, as this is all handled by lower level firmware.

Some of the custom properties (e.g. tip sense debounce) should technically 
work if we teach the driver how to set them at probe time, however given that 
the chip is almost certainly never going to be used outside of the Apple 
Silicon context we went with static values that were first observed under macOS 
and then optimised once we had enough of a working stack to play around in 
Linux. The values that go into the registers for each property appear to be 
identical to CS42L42, so it should be possible to implement at least some of 
these properties if that is what is preferred.

On Tuesday 22 October 2024 8:15:50 AM AEST Mark Brown wrote:
> I don't know if the Cirrus people who are listed as maintainers of that
> binding might have concerns about doing things that acknowledge this
> particular part.

I suspect, based on the past behaviour of Apple vendors including Cirrus 
themselves, that they either won't or can't publicly acknowledge the existence 
of this chip. Given this, and the fact that the chip will almost certainly 
never be used on any other platform, it is probably better for everyone 
involved if we cut them out of the loop.

Regards,
James
Rob Herring (Arm) Oct. 24, 2024, 2:59 p.m. UTC | #4
On Sun, 20 Oct 2024 00:47:31 +1000, James Calligeros wrote:
> From: Martin Povišer <povik+lin@cutebit.org>
> 
> CS42L84 is a headphone jack codec made by Cirrus Logic and seen in Apple
> computer models starting with 2021 Macbook Pros. It is not a publicly
> documented part. To a degree the part is similar to the public CS42L42.
> (The L84 superseded L83 seen in earlier Apple models, and the L83 was
> pretty much the same as L42.)
> 
> Signed-off-by: Martin Povišer <povik+lin@cutebit.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> ---
>  .../bindings/sound/cirrus,cs42l84.yaml   | 56 +++++++++++++++++++++++++
>  MAINTAINERS                              |  1 +
>  2 files changed, 57 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Mark Brown Oct. 24, 2024, 3:09 p.m. UTC | #5
On Sun, Oct 20, 2024 at 12:47:31AM +1000, James Calligeros wrote:
> From: Martin Povišer <povik+lin@cutebit.org>
> 
> CS42L84 is a headphone jack codec made by Cirrus Logic and seen in Apple
> computer models starting with 2021 Macbook Pros. It is not a publicly
> documented part. To a degree the part is similar to the public CS42L42.
> (The L84 superseded L83 seen in earlier Apple models, and the L83 was
> pretty much the same as L42.)

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..7f8338e8ae369bc529ac3cf35041d5a7b9f3e6d1
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml
@@ -0,0 +1,56 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/cirrus,cs42l84.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS42L84 audio CODEC
+
+maintainers:
+  - Martin Povišer <povik+lin@cutebit.org>
+
+description: |
+  The CS42L84 is a headphone jack codec made by Cirrus Logic and embedded
+  in personal computers sold by Apple. It was first seen in 2021 Macbook
+  Pro models. It has stereo DAC for playback, mono ADC for capture, and
+  is somewhat similar to CS42L42 but with a different regmap.
+
+properties:
+  compatible:
+    enum:
+      - cirrus,cs42l84
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  '#sound-dai-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      jack_codec: codec@4b {
+          compatible = "cirrus,cs42l84";
+          reg = <0x4b>;
+          reset-gpios = <&pinctrl_nub 4 GPIO_ACTIVE_LOW>;
+          interrupts-extended = <&pinctrl_ap 180 IRQ_TYPE_LEVEL_LOW>;
+          #sound-dai-cells = <0>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index c1a2c296446c0724a0c6e70c845e5e5e1e693fd5..f5f85714dc4e8ca9c60b3f6963b2cec1ea33fdd0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2132,6 +2132,7 @@  L:	asahi@lists.linux.dev
 L:	linux-sound@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/sound/adi,ssm3515.yaml
+F:	Documentation/devicetree/bindings/sound/cirrus,cs42l84.yaml
 F:	Documentation/devicetree/bindings/sound/apple,*
 F:	sound/soc/apple/*
 F:	sound/soc/codecs/cs42l83-i2c.c