From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Ujfalusi Subject: Re: [PATCH] ASoC: core: Clean up DAPM before the card debugfs Date: Mon, 22 Aug 2016 16:32:21 +0300 Message-ID: <0aff36cb-63cd-baa0-5a97-49212ee0dcf6@ti.com> References: <1471545449-29516-1-git-send-email-broonie@kernel.org> <20160818232642.GF1041@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from comal.ext.ti.com (comal.ext.ti.com [198.47.26.152]) by alsa0.perex.cz (Postfix) with ESMTP id 305DF2657B6 for ; Mon, 22 Aug 2016 15:32:24 +0200 (CEST) In-Reply-To: <20160818232642.GF1041@n2100.armlinux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Russell King - ARM Linux , Mark Brown Cc: alsa-devel@alsa-project.org, Liam Girdwood List-Id: alsa-devel@alsa-project.org On 08/19/16 02:26, Russell King - ARM Linux wrote: > On Thu, Aug 18, 2016 at 07:37:29PM +0100, Mark Brown wrote: >> Both the card and DAPM cleanups recursively delete their debugfs >> directories. Since the DAPM debugfs subdirectory for the card is >> located within the card debugfs this means we end up trying to double >> free the DAPM subdirectory. Reorder the cleanup to free the card >> debugfs after we've cleaned up DAPM and it has deleted its own >> subdirectory. >> >> Reported-by: Russell King - ARM Linux >> Signed-off-by: Mark Brown >> --- >> >> Not tested at all yet, just about done for today. > = > Thanks - that appears to solve the oops on rmmod of > snd-soc-omap-abe-twl6040! > = > Tested-by: Russell King > = > So this allows me to test a bit more... and there's more bad news. > On re-inserting this module... > = > twl6040 0-004b: Unable to sync registers 0xc-0xd. -121 > genirq: Flags mismatch irq 242. 00000004 (McPDM) vs. 00000004 (McPDM) > omap-mcpdm 40132000.mcpdm: Request for IRQ failed > omap-mcpdm 40132000.mcpdm: ASoC: failed to probe DAI 40132000.mcpdm: -16 > omap-abe-twl6040 sound: ASoC: failed to instantiate card -16 > omap-abe-twl6040 sound: snd_soc_register_card() failed: -16 > omap-abe-twl6040: probe of sound failed with error -16 > = > Looks like removing and re-inserting these modules has never been > tested. :( And oh my... > = > static int omap_mcpdm_probe(struct snd_soc_dai *dai) > { > ret =3D devm_request_irq(mcpdm->dev, mcpdm->irq, omap_mcpdm_irq_h= andler, > 0, "McPDM", (void *)mcpdm); > = > static struct snd_soc_dai_driver omap_mcpdm_dai =3D { > .probe =3D omap_mcpdm_probe, > .remove =3D omap_mcpdm_remove, > = > Using devm_* stuff in a context where it doesn't get automatically > freed when the DAI driver is unbound, and nothing in omap_mcpdm_remove() > to undo that request. > = > I suspect that isn't the full story though, because of the regmap > complaint (which I've not even looked at yet.) > = > At the moment, omap_mcpdm_probe() should _not_ be making use of any > devm_* calls, and omap_mcpdm_remove() should be _explicitly_ releasing > everything that was claimed in omap_mcpdm_probe() because ASoC does > nothing with devres groups. The alternative solution here is that > ASoC gains devres group support so resources claimed in the DAI > driver .probe function can be automatically released in the same way > as happens with normal driver model probe/release and component > helper bind/unbind. > = > Maybe we should have a test mode in the kernel where every device > goes through a bind-unbind-rebind sequence during boot to test more > of these paths... Ouch, we have only tested modules when they are all removed. I will fix it = asap. -- = P=E9ter