| 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 |
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.
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.
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.
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?
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.
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?
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 --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>; + }; + +...
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