[PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers

Markus Probst posted 4 patches 1 month, 2 weeks ago
[PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers
Posted by Markus Probst 1 month, 2 weeks ago
From: Kari Argillander <kari.argillander@gmail.com>

The serdev core has an internal is_serdev_device() helper, but it was
not accessible to drivers. Make it public by declaring it in serdev.h
and exporting the symbol so that modular serdev drivers can rely on it
instead of duplicating type checks.

This allows example future Rust serdev abstraction to have

    TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>

That way using bus is easy for other substystems. Also some other
subsystems expose similar function:

  - bool is_usb_device(const struct device *dev)
  - bool dev_is_pci(const struct device *dev)

Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
---
 drivers/tty/serdev/core.c | 3 ++-
 include/linux/serdev.h    | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index b33e708cb245..1f6bf8e826d8 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -69,10 +69,11 @@ static const struct device_type serdev_device_type = {
 	.release	= serdev_device_release,
 };
 
-static bool is_serdev_device(const struct device *dev)
+bool is_serdev_device(const struct device *dev)
 {
 	return dev->type == &serdev_device_type;
 }
+EXPORT_SYMBOL_GPL(is_serdev_device);
 
 static void serdev_ctrl_release(struct device *dev)
 {
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 34562eb99931..0043b6cc6d01 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -116,6 +116,8 @@ static inline struct serdev_controller *to_serdev_controller(struct device *d)
 	return container_of(d, struct serdev_controller, dev);
 }
 
+bool is_serdev_device(const struct device *dev);
+
 static inline void *serdev_device_get_drvdata(const struct serdev_device *serdev)
 {
 	return dev_get_drvdata(&serdev->dev);

-- 
2.51.2
Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers
Posted by Greg Kroah-Hartman 1 month, 2 weeks ago
On Sat, Dec 20, 2025 at 06:44:05PM +0000, Markus Probst wrote:
> From: Kari Argillander <kari.argillander@gmail.com>
> 
> The serdev core has an internal is_serdev_device() helper, but it was
> not accessible to drivers. Make it public by declaring it in serdev.h
> and exporting the symbol so that modular serdev drivers can rely on it
> instead of duplicating type checks.
> 
> This allows example future Rust serdev abstraction to have
> 
>     TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>

But why is that going to be needed?

> That way using bus is easy for other substystems. Also some other
> subsystems expose similar function:
> 
>   - bool is_usb_device(const struct device *dev)
>   - bool dev_is_pci(const struct device *dev)

Yes, and usually that's not a good idea, unless you have a bus with
multiple types of devices on it.  I don't think serdev has that, does
it?

Only under special circumstances should this be required, so I'm curious
as to why you would ever have a pointer to a struct device and not
"know" that it is of this type?  Who is passing that to you?

thanks,

greg k-h
Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers
Posted by Markus Probst 1 month, 2 weeks ago
On Sun, 2025-12-21 at 17:10 +0100, Greg Kroah-Hartman wrote:
> On Sat, Dec 20, 2025 at 06:44:05PM +0000, Markus Probst wrote:
> > From: Kari Argillander <kari.argillander@gmail.com>
> > 
> > The serdev core has an internal is_serdev_device() helper, but it was
> > not accessible to drivers. Make it public by declaring it in serdev.h
> > and exporting the symbol so that modular serdev drivers can rely on it
> > instead of duplicating type checks.
> > 
> > This allows example future Rust serdev abstraction to have
> > 
> >     TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>
> 
> But why is that going to be needed?
> 
> > That way using bus is easy for other substystems. Also some other
> > subsystems expose similar function:
> > 
> >   - bool is_usb_device(const struct device *dev)
> >   - bool dev_is_pci(const struct device *dev)
> 
> Yes, and usually that's not a good idea, unless you have a bus with
> multiple types of devices on it.  I don't think serdev has that, does
> it?
> 
> Only under special circumstances should this be required, so I'm curious
> as to why you would ever have a pointer to a struct device and not
> "know" that it is of this type?  Who is passing that to you?
For example, the pwm rust abstraction currently only provides a
`device::Device<device::Bound>` reference in callbacks [1]. If we want
to write data to the serial device in one of the pwm callbacks, we need
to convert the `device::Device<device::Bound>` reference to
`serdev::Device<device::Bound>`. The TryFrom implementation provides a
*safe* abstraction.

At least in the pwm example, this could/should be circumvented with the
`AsBusDevice` trait [2].

But this technically also applies to all other bus device abstractions
and they also have a `TryFrom<&device::Device<Ctx>>` implementation, so
with this it would be consistent across all rust bus device
abstractions.

Thanks
- Markus Probst

[1] https://rust.docs.kernel.org/kernel/pwm/trait.PwmOps.html

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/rust/kernel/device.rs?id=e4addc7cc2dfcc19f1c8c8e47f3834b22cb21559

> 
> thanks,
> 
> greg k-h
Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers
Posted by Greg Kroah-Hartman 1 month, 2 weeks ago
On Sun, Dec 21, 2025 at 04:28:11PM +0000, Markus Probst wrote:
> On Sun, 2025-12-21 at 17:10 +0100, Greg Kroah-Hartman wrote:
> > On Sat, Dec 20, 2025 at 06:44:05PM +0000, Markus Probst wrote:
> > > From: Kari Argillander <kari.argillander@gmail.com>
> > > 
> > > The serdev core has an internal is_serdev_device() helper, but it was
> > > not accessible to drivers. Make it public by declaring it in serdev.h
> > > and exporting the symbol so that modular serdev drivers can rely on it
> > > instead of duplicating type checks.
> > > 
> > > This allows example future Rust serdev abstraction to have
> > > 
> > >     TryFrom<&device::Device<Ctx>> for &serdev::Device<Ctx>
> > 
> > But why is that going to be needed?
> > 
> > > That way using bus is easy for other substystems. Also some other
> > > subsystems expose similar function:
> > > 
> > >   - bool is_usb_device(const struct device *dev)
> > >   - bool dev_is_pci(const struct device *dev)
> > 
> > Yes, and usually that's not a good idea, unless you have a bus with
> > multiple types of devices on it.  I don't think serdev has that, does
> > it?
> > 
> > Only under special circumstances should this be required, so I'm curious
> > as to why you would ever have a pointer to a struct device and not
> > "know" that it is of this type?  Who is passing that to you?
> For example, the pwm rust abstraction currently only provides a
> `device::Device<device::Bound>` reference in callbacks [1]. If we want
> to write data to the serial device in one of the pwm callbacks, we need
> to convert the `device::Device<device::Bound>` reference to
> `serdev::Device<device::Bound>`. The TryFrom implementation provides a
> *safe* abstraction.

While I like the feeling of *safe* you can mark it as *safe* as the
driver core can NOT give you a callback of a pointer that is not of that
type.

That's how the C code works today, and is why the C code does not need
this function exported.  The rust code should do the same thing.

thanks,

greg k-h
Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers
Posted by Danilo Krummrich 1 month, 2 weeks ago
On Sun Dec 21, 2025 at 5:28 PM CET, Markus Probst wrote:
> On Sun, 2025-12-21 at 17:10 +0100, Greg Kroah-Hartman wrote:
>> Yes, and usually that's not a good idea, unless you have a bus with
>> multiple types of devices on it.  I don't think serdev has that, does
>> it?
>> 
>> Only under special circumstances should this be required, so I'm curious
>> as to why you would ever have a pointer to a struct device and not
>> "know" that it is of this type?  Who is passing that to you?
> For example, the pwm rust abstraction currently only provides a
> `device::Device<device::Bound>` reference in callbacks [1]. If we want
> to write data to the serial device in one of the pwm callbacks, we need
> to convert the `device::Device<device::Bound>` reference to
> `serdev::Device<device::Bound>`. The TryFrom implementation provides a
> *safe* abstraction.
>
> At least in the pwm example, this could/should be circumvented with the
> `AsBusDevice` trait [2].

Yes, class device implementations should start using AsBusDevice, so you don't
need the TryFrom upcast.

> But this technically also applies to all other bus device abstractions
> and they also have a `TryFrom<&device::Device<Ctx>>` implementation, so
> with this it would be consistent across all rust bus device
> abstractions.

It should only be PCI and platform and they have it for a reason ()which is the
one Greg already mentioned, i.e. when you potentially have multiple device types
on a bus).

The concrete use-case that motivated adding those for PCI and platform is when
they interact with the auxiliary bus: Imagine a driver that supports devices
from the platform bus and the PCI bus and exports an auxiliary device.

In this case, when you call back into the parent driver from the auxiliary
device the parent driver has to upcast.

This exact constellation is what is implemented by nova-core and nova-drm.

However, outside of such a constellation we should avoid doing upcasts with
TryFrom and make the correct type available in the first place.

- Danilo
Re: [PATCH RFC 1/4] serdev: Export internal is_serdev_device() for drivers
Posted by Danilo Krummrich 1 month, 2 weeks ago
On Sat Dec 20, 2025 at 7:44 PM CET, Markus Probst wrote:
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index b33e708cb245..1f6bf8e826d8 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -69,10 +69,11 @@ static const struct device_type serdev_device_type = {
>  	.release	= serdev_device_release,
>  };
>  
> -static bool is_serdev_device(const struct device *dev)
> +bool is_serdev_device(const struct device *dev)
>  {
>  	return dev->type == &serdev_device_type;
>  }
> +EXPORT_SYMBOL_GPL(is_serdev_device);
>  
>  static void serdev_ctrl_release(struct device *dev)
>  {
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index 34562eb99931..0043b6cc6d01 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -116,6 +116,8 @@ static inline struct serdev_controller *to_serdev_controller(struct device *d)
>  	return container_of(d, struct serdev_controller, dev);
>  }
>  
> +bool is_serdev_device(const struct device *dev);
> +
>  static inline void *serdev_device_get_drvdata(const struct serdev_device *serdev)
>  {
>  	return dev_get_drvdata(&serdev->dev);

Besides what I mentioned in [1] (which should make the patch obsolete anyways),
I want to point out that such cases should be solved with a helper in
rust/helpers/ instead.

[1] https://lore.kernel.org/lkml/DF42RR0I52L3.1JET4R0KFDPPH@kernel.org/