| 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
| ||
|
Message-ID: <CAMRc=Mc-WebsQZ3jt2xirioNMticiWj9PJ3fsPTXGCeJ1iTLRg@mail.gmail.com>
Date: Thu, 27 Nov 2025 06:32:04 -0800
From: Bartosz Golaszewski <brgl@...ev.pl>
To: manivannan.sadhasivam@....qualcomm.com
Cc: 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>,
Manivannan Sadhasivam <mani@...nel.org>, 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>,
Bartosz Golaszewski <brgl@...ev.pl>
Subject: Re: [PATCH v2 02/10] serdev: Add serdev device based driver match support
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.
Which also makes me think that maybe we should finally think about a generic,
fwnode-based device driver matching in the driver model...
Bartosz
> static int serdev_device_match(struct device *dev, const struct device_driver *drv)
> {
> if (!is_serdev_device(dev))
> return 0;
>
> - /* TODO: platform matching */
> + if (serdev_driver_match_device(dev, drv))
> + return 1;
> +
> if (acpi_driver_match_device(dev, drv))
> return 1;
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 6077972e8b45..70c54c4bedba 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -976,4 +976,11 @@ struct coreboot_device_id {
> kernel_ulong_t driver_data;
> };
>
> +#define SERDEV_NAME_SIZE 32
> +
> +struct serdev_device_id {
> + const char name[SERDEV_NAME_SIZE];
> + kernel_ulong_t driver_data;
> +};
> +
> #endif /* LINUX_MOD_DEVICETABLE_H */
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index ecde0ad3e248..aca92e0ee6e7 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -39,6 +39,7 @@ struct serdev_device_ops {
> * @ops: Device operations.
> * @write_comp Completion used by serdev_device_write() internally
> * @write_lock Lock to serialize access when writing data
> + * @id: serdev device ID entry
> */
> struct serdev_device {
> struct device dev;
> @@ -47,6 +48,7 @@ struct serdev_device {
> const struct serdev_device_ops *ops;
> struct completion write_comp;
> struct mutex write_lock;
> + const struct serdev_device_id *id;
> };
>
> #define to_serdev_device(d) container_of_const(d, struct serdev_device, dev)
> @@ -55,11 +57,13 @@ struct serdev_device {
> * struct serdev_device_driver - serdev slave device driver
> * @driver: serdev device drivers should initialize name field of this
> * structure.
> + * @id_table: serdev device ID table
> * @probe: binds this driver to a serdev device.
> * @remove: unbinds this driver from the serdev device.
> */
> struct serdev_device_driver {
> struct device_driver driver;
> + const struct serdev_device_id *id_table;
> int (*probe)(struct serdev_device *);
> void (*remove)(struct serdev_device *);
> };
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index d3d00e85edf7..c1bfa8eddc4d 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -280,5 +280,8 @@ int main(void)
> DEVID(coreboot_device_id);
> DEVID_FIELD(coreboot_device_id, tag);
>
> + DEVID(serdev_device_id);
> + DEVID_FIELD(serdev_device_id, name);
> +
> return 0;
> }
>
> --
> 2.48.1
>
>
>
Powered by blists - more mailing lists