Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1597644455-8216-1-git-send-email-jiaxin.yu@mediatek.com>
 <1597644455-8216-3-git-send-email-jiaxin.yu@mediatek.com> <CA+Px+wXSbGLb+AZnF8ETRycRUVjqk4xacm5DH6MzuMw0vh6Wzg@mail.gmail.com>
 <CA+Px+wUMXoSL6w0wBduE7obJRWgCteeT8=_=U=8LR34JKTTGZA@mail.gmail.com>
 <20200819103730.GB5441@sirena.org.uk> <CA+Px+wUV89KO8JJd3+HpOrgFRSc7sdg-DBW44C31262Qx9NzVg@mail.gmail.com>
 <20200819194005.GC38371@sirena.org.uk>
In-Reply-To: <20200819194005.GC38371@sirena.org.uk>
From:   Tzung-Bi Shih <tzungbi@google.com>
Date:   Mon, 7 Sep 2020 21:37:12 +0800
Message-ID: <CA+Px+wWMH6iUzFq0g4BFC5qA993r9UBobmNAyS2ie+xDwci-yg@mail.gmail.com>
Subject: Re: [PATCH v5 2/2] dt-bindings: mediatek: mt6359: add codec document
To:     Mark Brown <broonie@kernel.org>
Cc:     Jiaxin Yu <jiaxin.yu@mediatek.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Takashi Iwai <tiwai@suse.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        ALSA development <alsa-devel@alsa-project.org>,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org, howie.huang@mediatek.com,
        eason.yen@mediatek.com, shane.chien@mediatek.com,
        bicycle.tsai@mediatek.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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?