linux-kernel - Re: [PATCH] ASoC: Intel: Handle device properties with software node API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YHadFnjWy2zjwxo2@kuha.fi.intel.com>
Date:   Wed, 14 Apr 2021 10:43:18 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
Cc:     Cezary Rojewski <cezary.rojewski@...el.com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Jie Yang <yang.jie@...ux.intel.com>,
        linux-kernel@...r.kernel.org, Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Brown <broonie@...nel.org>, alsa-devel@...a-project.org,
        Bard Liao <yung-chuan.liao@...ux.intel.com>
Subject: Re: [PATCH] ASoC: Intel: Handle device properties with software node
 API

On Tue, Apr 13, 2021 at 10:47:49AM -0500, Pierre-Louis Bossart wrote:
> 
> 
> On 4/13/21 9:05 AM, Heikki Krogerus wrote:
> > On Tue, Apr 13, 2021 at 03:20:45PM +0300, Heikki Krogerus wrote:
> > > On Mon, Apr 12, 2021 at 03:36:20PM -0500, Pierre-Louis Bossart wrote:
> > > > I took the code and split it in two for BYT/CHT (modified to remove devm_)
> > > > and SoundWire parts (added as is).
> > > > 
> > > > https://github.com/thesofproject/linux/pull/2810
> > > > 
> > > > Both cases result in a refcount error on device_remove_sof when removing the
> > > > platform device. I don't understand the code well enough to figure out what
> > > > happens, but it's likely a case of the software node being removed twice?
> > > 
> > > Right. Because you are injecting the node to an already existing
> > > device, the node does not get linked with the device in sysfs. That
> > > would increment the reference count in a normal case. It all happens
> > > in the function software_node_notify(). Driver core calls it when a
> > > device is added and also when it's removed. In this case it is only
> > > called when it's removed.
> > > 
> > > I think the best way to handle this now is to simply not decrementing
> > > the ref count when you add the properties, so don't call
> > > fwnode_handle_put() there (but add a comment explaining why you are
> > > not calling it).
> > 
> > No, sorry... That's just too hacky. Let's not do that after all.
> > 
> > We can also fix this in the software node code. I'm attaching a patch
> > that should make it possible to inject the software nodes also
> > afterwards safely. This is definitely also not without its problems,
> > but we can go with that if it works. Let me know.
> 
> I tested manually on bytcr w/ RT5640 and used the SOF CI farm to test the
> SoundWire cases, I don't see any issues with your patch. The refcount issue
> is gone and the module load/unload cycles don't report any problems.
> 
> Would you queue it for 5.13-rc1, or is this too late already?

I'll send it out now. Let's see what happens.

thanks,

> > > For a better solution you would call device_reprobe() after you have
> > > injected the software node, but before that you need to modify
> > > device_reprobe() so it calls device_platform_notify() (which it really
> > > should call in any case). But this should probable be done later,
> > > separately.
> > > 
> > > thanks,
> > > 
> > > P.S.
> > > 
> > > Have you guys considered the possibility of describing the connections
> > > between all these components by using one of the methods that we now
> > > have for that in kernel, for example device graph? It can now be
> > > used also with software nodes (OF graph and ACPI device graph are of
> > > course already fully supported).
> 
> I must admit I've never heard of a 'device graph'. Any pointers or APIs you
> can think of?
> It's a good comment since we are planning to rework the SOF clients and
> machine driver handling.

-- 
heikki