Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()

Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()

[Cristian Marussi]

On Wed, Jan 14, 2026 at 12:28:43AM +0800, Gui-Dong Han wrote:
> Currently, driver_match_device() is called from three sites. One site
> (__device_attach_driver) holds device_lock(dev), but the other two
> (bind_store and __driver_attach) do not. This inconsistency means that
> bus match() callbacks are not guaranteed to be called with the lock
> held.
> 
> Fix this by introducing driver_match_device_locked(), which guarantees
> holding the device lock using a scoped guard. Replace the unlocked calls
> in bind_store() and __driver_attach() with this new helper. Also add a
> lock assertion to driver_match_device() to enforce this guarantee.

Hi,

it has been reported by Gatien (in CC) that this break the SCMI OPTEE
transport.

Moreover I still have to verify, BUT I think this also breaks SCMI
Virtio transport since both call platform_driver_register() during their
probe, since a few years ago the SCMI transports have been reworked to be
standalone full-fledged drivers.

I'll have a look in the next days if we can cope with this.

Thanks,
Cristian

Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()

[Danilo Krummrich]

On Wed Feb 25, 2026 at 9:19 PM CET, Cristian Marussi wrote:
> it has been reported by Gatien (in CC) that this break the SCMI OPTEE
> transport.
>
> Moreover I still have to verify, BUT I think this also breaks SCMI
> Virtio transport since both call platform_driver_register() during their
> probe, since a few years ago the SCMI transports have been reworked to be
> standalone full-fledged drivers.

I've had a quick look and I'm pretty sure that both transports/virtio.c and
transports/optee.c are broken.

Both cases look identical and I think the fix should be as trivial as moving
platform_driver_register() into module_init().

> I'll have a look in the next days if we can cope with this.

I already fixed those three [1,2,3]. If you can provide a patch for both of
them, that'd be great. Otherwise, please let me know if you want me to send
something.

Thanks,
Danilo

[1] https://lore.kernel.org/driver-core/20260121141215.29658-1-dakr@kernel.org/
[2] https://lore.kernel.org/driver-core/20260123133614.72586-1-dakr@kernel.org/
[3] https://lore.kernel.org/driver-core/20260212235842.85934-1-dakr@kernel.org/

Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()

[Gatien CHEVALLIER]

On 2/25/26 21:38, Danilo Krummrich wrote:
> On Wed Feb 25, 2026 at 9:19 PM CET, Cristian Marussi wrote:
>> it has been reported by Gatien (in CC) that this break the SCMI OPTEE
>> transport.
>>
>> Moreover I still have to verify, BUT I think this also breaks SCMI
>> Virtio transport since both call platform_driver_register() during their
>> probe, since a few years ago the SCMI transports have been reworked to be
>> standalone full-fledged drivers.
> 
> I've had a quick look and I'm pretty sure that both transports/virtio.c and
> transports/optee.c are broken.
> 
> Both cases look identical and I think the fix should be as trivial as moving
> platform_driver_register() into module_init().
> 

Hello,

(Regarding OP-TEE SCMI transport)

It may not be that trivial because unconditionally moving it to
module_init() would not guarantee that the TEE bus driver probes
before the platform device driver. I think there's a bit more to it.

The platform device driver, as is, is not functional whilst the
TEE services are not available. Therefore, it could falsely fail to
probe if probed before. We may consider some sort of API to know if
the services are available?

Gatien

>> I'll have a look in the next days if we can cope with this.
> 
> I already fixed those three [1,2,3]. If you can provide a patch for both of
> them, that'd be great. Otherwise, please let me know if you want me to send
> something.
> 
> Thanks,
> Danilo
> 
> [1] https://lore.kernel.org/driver-core/20260121141215.29658-1-dakr@kernel.org/
> [2] https://lore.kernel.org/driver-core/20260123133614.72586-1-dakr@kernel.org/
> [3] https://lore.kernel.org/driver-core/20260212235842.85934-1-dakr@kernel.org/

Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()

[Danilo Krummrich]

On Thu Feb 26, 2026 at 9:54 AM CET, Gatien CHEVALLIER wrote:
> It may not be that trivial because unconditionally moving it to
> module_init() would not guarantee that the TEE bus driver probes
> before the platform device driver. I think there's a bit more to it.
>
> The platform device driver, as is, is not functional whilst the
> TEE services are not available. Therefore, it could falsely fail to
> probe if probed before. We may consider some sort of API to know if
> the services are available?

Ok, so there are three drivers somehow involved:

  (1) module_tee_client_driver(scmi_optee_service_driver)

  (2) scmi_optee_driver with compatible = "linaro,scmi-optee"

  (3) scmi_driver with name = "arm-scmi"

(1) registeres (2) in probe(), and the only thing (2) does is allocating a
platform_device with name = "arm-scmi", such that (3) is probed.

So, to me it seems that the indirection through a fake platform device is
unnecessary and instead (3) should just have compatible = "linaro,scmi-optee"
and return -EPROBE_DEFER if (1) is not ready. Besides that, this seems like a
use-case for device links.

Thanks,
Danilo

Re: [PATCH v5] driver core: enforce device_lock for driver_match_device()

[Cristian Marussi]

On Thu, Feb 26, 2026 at 12:15:33PM +0100, Danilo Krummrich wrote:
> On Thu Feb 26, 2026 at 9:54 AM CET, Gatien CHEVALLIER wrote:

Hi,

I think my mail thread is a bit messed up...my previous reply is nowhere
to be found, so I will repeat here...

> > It may not be that trivial because unconditionally moving it to
> > module_init() would not guarantee that the TEE bus driver probes
> > before the platform device driver. I think there's a bit more to it.

@Gatien

Yes, it is just what I was going to reply...these SCMI transport drivers
basically have to probe first against their related subsystem like
OPTEE/Virtio and then, only when fully probed at this level, are fully
functional and can become a transport supplier for the SCMI core...
...registering the platform driver at the end of the OPTEE/VIRTIO probe
allowed to avoid any kind of EPROBE_DEFER handling that can come up
espcially when all of this is split into loadable modules..

...I have to review this in deep but I think I had a first version of these
transport rework that worked a bit differently...

> >
> > The platform device driver, as is, is not functional whilst the
> > TEE services are not available. Therefore, it could falsely fail to
> > probe if probed before. We may consider some sort of API to know if
> > the services are available?
> 
> Ok, so there are three drivers somehow involved:
> 
>   (1) module_tee_client_driver(scmi_optee_service_driver)
> 
>   (2) scmi_optee_driver with compatible = "linaro,scmi-optee"
> 
>   (3) scmi_driver with name = "arm-scmi"
> 
> (1) registeres (2) in probe(), and the only thing (2) does is allocating a
> platform_device with name = "arm-scmi", such that (3) is probed.
> 
> So, to me it seems that the indirection through a fake platform device is
> unnecessary and instead (3) should just have compatible = "linaro,scmi-optee"

@Danilo

That was the initial state, all the transports and related compatibles embedded
into the SCMI core stack...then we split out the transport as loadable standalone
drivers, since it seemed to be cleaner (and some vendor also asked for by that
time...).

	https://lore.kernel.org/arm-scmi/20240812173340.3912830-1-cristian.marussi@arm.com/

> and return -EPROBE_DEFER if (1) is not ready. Besides that, this seems like a
> use-case for device links.

We tried to unify the handling because other transports (mailbox/smc) do
NOT rely on 3rd party subsystem to be initialized and this approach
indeed simplified all because you dont need any DEFER simply because
the SCMI core probing will be kicked off ONLY after at least a transport
is successfully probed and initialized...there is a devlink indeed later
that classifies the used transport as a supplier of the core SCMI
arm-scmi driver...

I have to recall the full details especially of the reqs that led to
this design bit I suppose we should at this point reintroduce a bit of
EDEFER...

Thanks,
Cristian