linux-kernel - Re: [PATCH v2 02/10] serdev: Add serdev device based driver match support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <fwzmob6ez7c6xbakcd4rq2icp7mdwgdvimss3zybb4ivdds3uo@mwguaz7rekjc>
Date: Tue, 30 Dec 2025 13:26:44 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: manivannan.sadhasivam@....qualcomm.com, 
	Manivannan Sadhasivam via B4 Relay <devnull+manivannan.sadhasivam.oss.qualcomm.com@...nel.org>, linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kbuild@...r.kernel.org, platform-driver-x86@...r.kernel.org, linux-pci@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-msm@...r.kernel.org, linux-bluetooth@...r.kernel.org, 
	linux-pm@...r.kernel.org, Stephan Gerhold <stephan.gerhold@...aro.org>, 
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>, Rob Herring <robh@...nel.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, 
	Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas.schier@...ux.dev>, 
	Hans de Goede <hansg@...nel.org>, Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	Mark Pearson <mpearson-lenovo@...ebb.ca>, "Derek J. Clark" <derekjohn.clark@...il.com>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Marcel Holtmann <marcel@...tmann.org>, Luiz Augusto von Dentz <luiz.dentz@...il.com>
Subject: Re: [PATCH v2 02/10] serdev: Add serdev device based driver match
 support

On Thu, Nov 27, 2025 at 06:32:04AM -0800, Bartosz Golaszewski wrote:
> On Tue, 25 Nov 2025 15:45:06 +0100, Manivannan Sadhasivam via B4 Relay
> <devnull+manivannan.sadhasivam.oss.qualcomm.com@...nel.org> said:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
> >
> > Add support to match serdev devices with serdev drivers based on the serdev
> > ID table defined in serdev_device_driver::id_table.
> >
> > The matching function, serdev_driver_match_device() uses the serdev device
> > name to match against the entries in serdev_device_driver::id_table.
> >
> > If there is no serdev id_table for the driver, then serdev_device_match()
> > will fallback to ACPI and DT based matching.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
> > ---
> >  drivers/tty/serdev/core.c         | 23 ++++++++++++++++++++++-
> >  include/linux/mod_devicetable.h   |  7 +++++++
> >  include/linux/serdev.h            |  4 ++++
> >  scripts/mod/devicetable-offsets.c |  3 +++
> >  4 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index b33e708cb245..2b5582cd5063 100644
> > --- a/drivers/tty/serdev/core.c
> > +++ b/drivers/tty/serdev/core.c
> > @@ -85,12 +85,33 @@ static const struct device_type serdev_ctrl_type = {
> >  	.release	= serdev_ctrl_release,
> >  };
> >
> > +static int serdev_driver_match_device(struct device *dev, const struct device_driver *drv)
> > +{
> > +	const struct serdev_device_driver *serdev_drv = to_serdev_device_driver(drv);
> > +	struct serdev_device *serdev = to_serdev_device(dev);
> > +	const struct serdev_device_id *id;
> > +
> > +	if (!serdev_drv->id_table)
> > +		return 0;
> > +
> > +	for (id = serdev_drv->id_table; id->name[0]; id++) {
> > +		if (!strcmp(dev_name(dev), id->name)) {
> > +			serdev->id = id;
> > +			return 1;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> 
> I don't know if Rob agrees with me but I would very much prefer to see
> software-node-based approach instead of an ID table matching.
> 
> Could you in the pwrseq driver, create a software node for the serdev device
> you allocate, set its "compatible" to "qcom,wcn7850-bt" and match against it
> here?
> 
> This has several benefits: if you ever need to pass more properties to the
> serdev devices, you already have a medium for that and you can also leave
> serdev_device_add() alone. You're comparing the entire name here - what if
> someone sets device's ID to some value and the name will be "WCN7850.2"?
> 
> You could also drop the serdev_id field from struct serdev_device. For matching
> you could even reuse the of_device_id from the device driver.
> 

I tried this approach and I really liked it since it gets rid of the yet-another
id_table for serdev (which I didn't like it btw). But there is one concern
though. We need a generic 'device_get_match_data' implementation for swnode.
While trying to implement it, I stumbled upon this patch [1] which does the same
for other usecase, but there was a disagreement on whether swnode should be used
for driver matching or not. For my usecase, I find it very useful and
reasonable, but Dmitry Torokhov believes otherwise.

Maybe I'll include this patch in the next version, CC Dmitry and see where it
goes.

> Which also makes me think that maybe we should finally think about a generic,
> fwnode-based device driver matching in the driver model...
> 

Yes, that would be useful too and will allow me to get rid of the custom
matching logic in serdev core.

- Mani

[1] https://lore.kernel.org/all/20240427203650.582989-1-sui.jingfeng@linux.dev

-- 
மணிவண்ணன் சதாசிவம்