netdev - Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <ZUlOGBxe4Dky-Qo1EtYMuS0kDRjYdkqex3qgSiFBkFwIdEUpHjsD2pcl3VvMPjD-ZAeqcP5P40AsSfHtv4fJ8Z8stUu4nwYFw0qt3vtf7yc=@protonmail.com>
Date: Wed, 25 Jun 2025 22:20:01 +0000
From: Yassine Oudjana <y.oudjana@...tonmail.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Lars-Peter Clausen <lars@...afoo.de>, Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konradybcio@...nel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas.schier@...ux.dev>, Alexander Sverdlin <alexander.sverdlin@...il.com>, Sean Nyekjaer <sean@...nix.com>, Javier Carrasco <javier.carrasco.cruz@...il.com>, Matti Vaittinen <mazziesaccount@...il.com>, Antoniu Miclaus <antoniu.miclaus@...log.com>, Ramona Gradinariu <ramona.gradinariu@...log.com>, "Yo-Jung (Leo) Lin" <0xff07@...il.com>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Neil Armstrong <neil.armstrong@...aro.org>, Barnabás Czémán <barnabas.czeman@...nlining.org>, Danila Tikhonov
	<danila@...xyga.com>, Antoni Pokusinski <apokusinski01@...il.com>, Vasileios Amoiridis <vassilisamir@...il.com>, Petar Stoykov <pd.pstoykov@...il.com>, shuaijie wang <wangshuaijie@...nic.com>, Yasin Lee <yasin.lee.x@...il.com>, "Borislav Petkov (AMD)" <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>, Tony Luck <tony.luck@...el.com>, Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>, Ingo Molnar <mingo@...nel.org>, Yassine Oudjana <yassine.oudjana@...il.com>, linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org, linux-arm-msm@...r.kernel.org, netdev@...r.kernel.org, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH 1/3] net: qrtr: Turn QRTR into a bus


On Sunday, April 6th, 2025 at 5:01 PM, Jonathan Cameron <jic23@...nel.org> wrote:

> On Sun, 06 Apr 2025 14:07:43 +0000
> Yassine Oudjana y.oudjana@...tonmail.com wrote:
> 
> > Implement a QRTR bus to allow for creating drivers for individual QRTR
> > services. With this in place, devices are dynamically registered for QRTR
> > services as they become available, and drivers for these devices are
> > matched using service and instance IDs.
> > 
> > In smd.c, replace all current occurences of qdev with qsdev in order to
> > distinguish between the newly added QRTR device which represents a QRTR
> > service with the existing QRTR SMD device which represents the endpoint
> > through which services are provided.
> > 
> > Signed-off-by: Yassine Oudjana y.oudjana@...tonmail.com
> 
> Hi Yassine
> 
> Just took a quick look through.
> 
> It might make more sense to do this with an auxiliary_bus rather
> than defining a new bus.
> 
> I'd also split out the renames as a precursor patch.
> 
> Various other comments inline.
> 
> Jonathan

<...>

> > + if (!del_server)
> > + return -ENOMEM;
> > +
> > + del_server->parent = qsdev;
> > + del_server->port = port;
> > +
> > + INIT_WORK(&del_server->work, qcom_smd_qrtr_del_device_worker);
> > + schedule_work(&del_server->work);
> > +
> > + return 0;
> > +}
> > +
> > +static int qcom_smd_qrtr_device_unregister(struct device *dev, void *data)
> > +{
> > + device_unregister(dev);
> 
> 
> One option that may simplify this is to do the device_unregister() handling
> a devm_action_or_reset() handler that is using the parent device as it's dev
> but unregistering the children. That way the unregister is called in the
> reverse order of setup and you only register a handler for those devices
> registered (rather walking children). I did this in the CXL pmu driver
> for instance.

Not sure I understand this correctly. This function is called for all children when
the parent (the bus) is removed in order to unregister them, so its called for all
registered devices under the parent. It's just a wrapper for device_unregister so
that it can be used with device_for_each_child. If I register a handler with
devm_add_action_or_reset using the parent device then it seems to me like I will
have to add a new function used as handler for that which in turn goes over the
children and unregisters them (we always unregister all children since the parent
will be no more) then I will only be adding an extra layer. I checked the CXL PMU
driver but I only found devm_add_action_or_reset used for cleaning up objects
associated with the device, not removing child devices.