Re: [RFC PATCH 2/3] ASoC: Intel: bdw-rt5677: fix module load/unload issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 05, 2020 at 12:08:57PM -0600, Pierre-Louis Bossart wrote:
> On 3/5/20 11:43 AM, Mark Brown wrote:
> > On Thu, Mar 05, 2020 at 08:51:03AM -0600, Pierre-Louis Bossart wrote:

> > > I *think* it's due to the need to use the codec component->dev, which is
> > > only available with the dailink callbacks - not on platform device probe
> > > which ends with the card registration.

> > Why do you have this need?  This is sounding a lot like the CODEC ought
> > to be requesting it...

> it's been that way since 2016 and the initial contribution. The Chrome folks
> might know more, I don't think anyone at Intel has worked on this code.

I'd have thought someone would've reviewed it on the way in?

> > > Well, the devm uses the component device, not the card device, so when
> > > removing the machine driver nothing should happen. The problem seems to be
> > > in the removal of the codec and component drivers.

> > Right, it's always a bad idea to do allocations with devm_ on a device
> > other than the one that you're currently working with - that clearly
> > leads to lifetime issues.

> that's precisely what I tried to correct.

In general the best (clearest, most robust) way to correct something
like this would be to continue to use devm_ but clean up the allocation
so that it's done by the device that is used.

> b) do you have any objections if we remove this devm_ use without trying to
> dig further into the gpio management. This is a 2015 product that we use to
> verify the SOF driver on Broadwell, not an Intel-owned device.

The main thing I'm missing with this is a coherent explanation of the
problem and how the changes proposed fix it.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux