From: Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
Mark Brown <broonie@kernel.org>,
Adam Thomson <Adam.Thomson.Opensource@diasemi.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
"Zhi, Yong" <yong.zhi@intel.com>
Subject: RE: [PATCH] ASoC: da7219: Fix general protection fault in da7219_register_dai_clks
Date: Thu, 30 Jul 2020 19:15:47 +0000 [thread overview]
Message-ID: <AM7PR10MB364075C56DA64BBA164A58A480710@AM7PR10MB3640.EURPRD10.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <6381232d-8226-fd0a-37bc-6bc42451b5e3@linux.intel.com>
On 30 July 2020 17:58, Pierre-Louis Bossart wrote:
> >>> When the card was uninstalled with modprobe -r, the da7219 codec was not
> >>> removed, only component da7219_remove() is invoked, do you suggest the
> >>> component driver probe and remove has to happen with da7219_i2_driver
> probe
> >>> and remove together? Thanks for the code review.
> >
> >> Well as far as I understand it the the devm_* allocated resources are tied to
> >> the i2c dev. If I'm correct then unless that's removed then those resources
> >> won't be freed. If this is a valid scenario then we would probably have to look
> >> at avoiding all devm_ calls in the da7219_probe() code as they wouldn't be
> >> released when doing what you are here.
> >
> >> Mark, what's your take on this? Am I missing something obvious?
> >
> > You're not missing anything, you shouldn't be doing devm allocations at
> > the CODEC level only at the device model level. I'm somewhat confused
> > why you would be registering clocks at the device model level TBH.
Ignorance on my part when I wrote the code I'd say.
>
> I am afraid we have quite a few codec drivers used in x86/ACPI platforms
> using devm_ clk functions at the component probe level...see e.g.:
>
> da7213, da7218, da7219, es8316, es8328, max98090, max98095, rt5514,
> rt5616, rt5640, rt5682, tlk320aic32x4
>
> some do even worse: nau8825, tlv320aic32x4 call devm_ functions in
> set_sysclk.
>
> The module load/unload tests used for SOF remove all the drivers, so as
> Adam was saying this should not happen if you remove the codec driver.
>
> It already took us quite a bit of effort to make sure all resources are
> properly allocated/released. We still have known issues when removing
> the platform driver only, mainly with HDaudio. I wasn't aware of this
> case for I2C codecs, I guess this just made the list of TODO cleanups
> even longer. D'oh!
Well, I'll make sure I at least sort the da721x drivers to rectify this
discrepancy and provide the correct solution. On the back of this I have also
spotted something else that looks stupid in the cold light day, so will deal
with that as well (relates to these updates).
next prev parent reply other threads:[~2020-07-30 19:16 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-30 14:04 [PATCH] ASoC: da7219: Fix general protection fault in da7219_register_dai_clks Yong Zhi
2020-07-30 15:53 ` Adam Thomson
2020-07-30 16:06 ` Zhi, Yong
2020-07-30 16:19 ` Adam Thomson
2020-07-30 16:28 ` Mark Brown
2020-07-30 16:58 ` Pierre-Louis Bossart
2020-07-30 19:15 ` Adam Thomson [this message]
2020-07-30 19:42 ` Zhi, Yong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM7PR10MB364075C56DA64BBA164A58A480710@AM7PR10MB3640.EURPRD10.PROD.OUTLOOK.COM \
--to=adam.thomson.opensource@diasemi.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=yong.zhi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).