diff mbox series

[v5,2/2] dt-bindings: mediatek: mt6359: add codec document

Message ID 1597644455-8216-3-git-send-email-jiaxin.yu@mediatek.com (mailing list archive)
State New, archived
Headers show
Series Add mediatek codec mt6359 driver | expand

Commit Message

Jiaxin Yu Aug. 17, 2020, 6:07 a.m. UTC
This patch adds MediaTek MT6359 codec document.

Signed-off-by: Jiaxin Yu <jiaxin.yu@mediatek.com>
---
 .../devicetree/bindings/sound/mt6359.yaml          | 68 ++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/mt6359.yaml

Comments

Tzung-Bi Shih Aug. 17, 2020, 7:29 a.m. UTC | #1
On Mon, Aug 17, 2020 at 2:08 PM Jiaxin Yu <jiaxin.yu@mediatek.com> wrote:
> +properties:
> +  compatible:
> +    const: mediatek,mt6359-sound

The compatible string has been removed since v3.

> +required:
> +  - compatible

The same here.

> +examples:
> +  - |
> +    mt6359codec: mt6359codec {
> +      compatible = "mediatek,mt6359-sound";

And the same here.
Tzung-Bi Shih Aug. 17, 2020, 8:11 a.m. UTC | #2
On Mon, Aug 17, 2020 at 3:29 PM Tzung-Bi Shih <tzungbi@google.com> wrote:
>
> On Mon, Aug 17, 2020 at 2:08 PM Jiaxin Yu <jiaxin.yu@mediatek.com> wrote:
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt6359-sound
>
> The compatible string has been removed since v3.
>
> > +required:
> > +  - compatible
>
> The same here.
>
> > +examples:
> > +  - |
> > +    mt6359codec: mt6359codec {
> > +      compatible = "mediatek,mt6359-sound";
>
> And the same here.

I misunderstood.  It still needs the compatible string to match the
corresponding driver.
Mark Brown Aug. 19, 2020, 10:37 a.m. UTC | #3
On Mon, Aug 17, 2020 at 04:11:03PM +0800, Tzung-Bi Shih wrote:
> On Mon, Aug 17, 2020 at 3:29 PM Tzung-Bi Shih <tzungbi@google.com> wrote:

> > > +required:
> > > +  - compatible

> > And the same here.

> I misunderstood.  It still needs the compatible string to match the
> corresponding driver.

No, it doesn't.  The MFD should be registering the platform device.
Tzung-Bi Shih Aug. 19, 2020, 3:42 p.m. UTC | #4
On Wed, Aug 19, 2020 at 6:38 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, Aug 17, 2020 at 04:11:03PM +0800, Tzung-Bi Shih wrote:
> > I misunderstood.  It still needs the compatible string to match the
> > corresponding driver.
>
> No, it doesn't.  The MFD should be registering the platform device.

I guess I see.  It lists the mfd_cell when calling
devm_mfd_add_devices() in drivers/mfd/mt6397-core.c.  It falls back to
use driver name and device name to match.  As long as the name
provided in mfd_cell matches the platform driver name, it works.

But I found struct mfd_cell also contains member .of_compatible.  What
is the difference if we use compatible string (as is) for this device
instead of falling back to use device name to match?
Mark Brown Aug. 19, 2020, 7:40 p.m. UTC | #5
On Wed, Aug 19, 2020 at 11:42:27PM +0800, Tzung-Bi Shih wrote:

> But I found struct mfd_cell also contains member .of_compatible.  What
> is the difference if we use compatible string (as is) for this device
> instead of falling back to use device name to match?

That's for binding the MFD subdevice to an OF node, you don't need to do
that for a device like this - you can just use the of_node of the parent
to get at the properties.
Tzung-Bi Shih Sept. 7, 2020, 1:37 p.m. UTC | #6
On Thu, Aug 20, 2020 at 3:40 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Aug 19, 2020 at 11:42:27PM +0800, Tzung-Bi Shih wrote:
>
> > But I found struct mfd_cell also contains member .of_compatible.  What
> > is the difference if we use compatible string (as is) for this device
> > instead of falling back to use device name to match?
>
> That's for binding the MFD subdevice to an OF node, you don't need to do
> that for a device like this - you can just use the of_node of the parent
> to get at the properties.

There is an issue we overlooked.  In sound/soc/codecs/mt6359.c,
mt6359_parse_dt() cannot read the DT properties
(https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/sound/soc/codecs/mt6359.c?h=for-next#n2640).

The original DTS is as following:
pmic {
  compatible = "mediatek,mt6359";

  mt6359codec: mt6359codec {
    compatible = "mediatek,mt6359-sound";  (1)
    mediatek,dmic-mode = <1>;
    mediatek,mic-type-0 = <2>;
  }
}
After removing the line at (1), mt6359_parse_dt() cannot read the DT properties.

The PMIC drivers/mfd/mt6397-core.c:
- "mediatek,mt6359"
- has the struct mfd_cell of mt6359-sound
- adds all mfd_cells via devm_mfd_add_devices().

The audio codec sound/soc/codecs/mt6359.c:
- "mediatek,mt6359-sound"


Here are a few options we can come out with.
1. adds back the compatible string in the DTS
2. gets of_node of parent in mt6359.c, and iterates all children nodes
to get the desired properties
3. parses all children nodes in the PMIC driver, and put them into
either platform_data or device properties
(https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/tree/include/linux/mfd/core.h?h=for-next#n77)
- The PMIC is common for several sub-devices.  It makes less sense to
handle subdevice specific logic in the common code.
4. others

Could you share with us what would you suggest for fixing the issue?
Mark Brown Sept. 7, 2020, 5:50 p.m. UTC | #7
On Mon, Sep 07, 2020 at 09:37:12PM +0800, Tzung-Bi Shih wrote:
> On Thu, Aug 20, 2020 at 3:40 AM Mark Brown <broonie@kernel.org> wrote:

> > That's for binding the MFD subdevice to an OF node, you don't need to do
> > that for a device like this - you can just use the of_node of the parent
> > to get at the properties.

> There is an issue we overlooked.  In sound/soc/codecs/mt6359.c,

> After removing the line at (1), mt6359_parse_dt() cannot read the DT properties.

> Here are a few options we can come out with.
> 1. adds back the compatible string in the DTS
> 2. gets of_node of parent in mt6359.c, and iterates all children nodes
> to get the desired properties

Do this, but instead of iterating all the child nodes just look for the
named CODEC node that you define in the bindings if you don't want to
put the properties in the root MFD node.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/mt6359.yaml b/Documentation/devicetree/bindings/sound/mt6359.yaml
new file mode 100644
index 0000000..a669b49
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/mt6359.yaml
@@ -0,0 +1,68 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/mt6359.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek MT6359 Codec Device Tree Bindings
+
+maintainers:
+  - Eason Yen <eason.yen@mediatek.com>
+  - Jiaxin Yu <jiaxin.yu@mediatek.com>
+  - Shane Chien <shane.chien@mediatek.com>
+
+description: |
+  The communication between MT6359 and SoC is through Mediatek PMIC wrapper.
+  For more detail, please visit Mediatek PMIC wrapper documentation.
+  Must be a child node of PMIC wrapper.
+
+properties:
+  compatible:
+    const: mediatek,mt6359-sound
+
+  mediatek,dmic-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Indicates how many data pins are used to transmit two channels of PDM
+      signal. 0 means two wires, 1 means one wire. Default value is 0.
+    enum:
+      - 0 # one wire
+      - 1 # two wires
+
+  mediatek,mic-type-0:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Specifies the type of mic type connected to adc0
+
+    enum:
+      - 0 # IDLE - mic in turn-off status
+      - 1 # ACC - analog mic with alternating coupling
+      - 2 # DMIC - digital mic
+      - 3 # DCC - analog mic with direct couping
+      - 4 # DCC_ECM_DIFF - analog electret condenser mic with differential mode
+      - 5 # DCC_ECM_SINGLE - analog electret condenser mic with single mode
+
+  mediatek,mic-type-1:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Specifies the type of mic type connected to adc1
+
+  mediatek,mic-type-2:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Specifies the type of mic type connected to adc2
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    mt6359codec: mt6359codec {
+      compatible = "mediatek,mt6359-sound";
+      mediatek,dmic-mode = <0>;
+      mediatek,mic-type-0 = <2>;
+    };
+
+...