Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   10 Jan 2019 10:16:03 +0900
Message-ID: <875zuxb9uy.wl-kuninori.morimoto.gx@renesas.com>
From:   Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
To:     Mark Brown <broonie@kernel.org>
Cc:     Jon Hunter <jonathanh@nvidia.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        linux-tegra@vger.kernel.org, Matthias Reichl <hias@horus.com>,
        alsa-devel@alsa-project.org,
        Marcel Ziswiler <marcel.ziswiler@toradex.com>,
        Takashi Iwai <tiwai@suse.com>, linux-kernel@vger.kernel.org,
        Marcel Ziswiler <marcel@ziswiler.com>
Subject: Re: [alsa-devel] [PATCH v1 3/3] ASoC: soc-core: fix platform name vs. of_node assignement
In-Reply-To: <20190109141439.GE10405@sirena.org.uk>
References: <20181218174040.k7u26vnnoplllnwb@camel2.lan>
        <952471da-b355-6471-6c19-5120d6704f81@nvidia.com>
        <87lg3vuc7p.wl-kuninori.morimoto.gx@renesas.com>
        <b5164c93-8be8-1c3d-9415-c895da6ad622@nvidia.com>
        <d1e41ed2-da7c-b46c-bc6d-64cf3e536771@nvidia.com>
        <865d2a3e-bf6b-1f30-1179-7e922c0d0641@nvidia.com>
        <87k1je38w7.wl-kuninori.morimoto.gx@renesas.com>
        <0660a471-3698-2059-4494-ad146518a4ed@nvidia.com>
        <20190109125340.GD10405@sirena.org.uk>
        <52f5f2cf-9260-f9c5-f73d-3c3d5debc86b@nvidia.com>
        <20190109141439.GE10405@sirena.org.uk>
User-Agent: Wanderlust/2.15.9 Emacs/24.5 Mule/6.0
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Hi Mark, Jon

> No. Offline you had suggested using kmalloc and not devm_kzalloc and so
> I was worried in that case about a memory leak. Right now I am only
> concerned about an invalid pointer that is not being handled correctly.

I'm sorry I was confused/misunderstood, kmalloc idea was wrong.

> I would like someone to explain what multi-platform means? Even if a
> soundcard supports multiple platforms, there is only one platform you
> are using at any time so ...
(snip)
> ... I don't understand why you would ever need a 'num_platform' as the
> machine driver just needs to understand which platform is using it at
> any given time. Right?

As Mark explained, "platform" on ALSA SoC means "DMA",
and we might have multiple DMA sound sysytem (= multi-platform) in the future.
Currently, all driver/sysytem is using single DMA for 1 sound card.

> > # So, I guess if your driver can switch to use
> > # snd_soc_init_platform style directly, your problem can gone ?
> 
> Yes that is an alternative and I can convert all the Tegra machine
> drivers to use this now. However, that will not solve the problem for
> non-Tegra devices and everyone will have to do this.

Yeah I agree.
But my concern is that the same problem happen on codec side too,
by same logic, because snd_soc_init_multicodec() is overwriting
dai_link too.
Your posted patch solved platform side only, I think.

> > >> Yes that is an alternative and I can convert all the Tegra machine
> > >> drivers to use this now. However, that will not solve the problem for
> > >> non-Tegra devices and everyone will have to do this.
> 
> > > We're going to have to go through another round of conversions that
> > > touch everything at some point no matter what :/
> 
> > Do you have a preference here? Do you think that we can fix-up the
> > soc-core or should I go ahead and migrate the Tegra machine driver to
> > workaround this issue now?
> 
> We're going to need to migrate Tegra regardless so it'd be good to do
> that whatever happens, I'm intending to try to properly review the patch
> today.

As I mentioned above, I think we have same issue on codec side too.
exchanging *platform to platform doesn't solve all issues.
And we need to exchange all driver again if we had multi-platform
support in the future (I don't know when it can happen though...)

My posted quick-patch can solve "dirty pointer" issue,
but it can't solve "memory leak" issue.
This issue will be solved if all driver can switch to
modern style, but it needs more time.
Are these correct ?

So, how about this ?

I will try to add snd_soc_dai_link_component support for CPU,
and switch all driver to use modern style for v5.1 (or v5.2 ?).
Until then, as temporary solution, we can use above quick-patch style.

And to avoid "memory leak crash" attach,
it temporary have bind dai_link limitation (max 5time?).
If it goes to max limitation, ALSA SoC doesn't allow to try again.
In such case, all related CPU/Codec driver need to rmmod/unbind,
and insmod/bind again.
Then, the limitation will be 0 cleared. You can try bind again.

It can solve "dirty pointer" issue, "memory leak" issue,
and "memory leak attack" issue.
The problem is that code can be dirty temporary.
But it will be removed if all driver can be swtich to modern style.

Best regards
---
Kuninori Morimoto