On Tue, Dec 16, 2025 at 12:08:38PM +0100, Uwe Kleine-König wrote:
Hello,
On Tue, Dec 16, 2025 at 01:08:38PM +0530, Sumit Garg wrote:
On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König u.kleine-koenig@baylibre.com wrote:
On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
Feel free to make the tee_bus_type private as the last patch in the series such that any followup driver follows this clean approach.
There is a bit more to do for that than I'm willing to invest. With my patch series applied `tee_bus_type` is still used in drivers/tee/optee/device.c and drivers/tee/tee_core.c.
Oh I see, I guess we need to come with some helpers around device register/unregister from TEE subsystem as well. Let's plan that for a followup patch-set, I don't want this patch-set to be bloated more.
Don't consider me in for that. But it sounds like a nice addition.
No worries, the current cleanup is fine for now.
Maybe it's sensible to merge these two files into a single one.
It's not possible as the design for TEE bus is to have TEE implementation drivers like OP-TEE, AMD-TEE, TS-TEE, QTEE and so on to register devices on the bus.
So only OP-TEE uses the bus for devices and the other *-TEE don't. Also sounds like something worth to be fixed.
The TEE bus is common for all the TEE implementation drivers which need to support kernel TEE client drivers. I am aware there will be QTEE and TS-TEE from past discussion that they will be exposing devices on the TEE bus.
The things I wonder about additionally are:
- if CONFIG_OPTEE=n and CONFIG_TEE=y|m the tee bus is only used for drivers but not devices.
Yeah since the devices are rather added by the TEE implementation driver.
- optee_register_device() calls device_create_file() on &optee_device->dev after device_register(&optee_device->dev). (Attention half-knowledge!) I think device_create_file() should not be called on an already registered device (or you have to send a uevent afterwards). This should probably use type attribute groups. (Or the need_supplicant attribute should be dropped as it isn't very useful. This would maybe be considered an ABI change however.)
The reasoning for this attribute should be explained by commit: 7269cba53d90 ("tee: optee: Fix supplicant based device enumeration"). In summary it's due to a weird dependency for devices we have with the user-space daemon: tee-supplicant.
From reading that once I don't understand it. (But no need to explain :-)
Still the file should better be added before device_add() is called.
Noted, let me see if I can get to fix this until someone jumps in before me.
- Why does optee_probe() in drivers/tee/optee/smc_abi.c unregister all optee devices in its error path (optee_unregister_devices())?
This is mostly to take care of if any device got registered before the failure occured. Let me know if you have a better way to address that.
Without understanding the tee stuff, I'd say: Don't bother and only undo the things that probe did before the failure.
True, but this is special case where if there is any leftover device registered from the TEE implementation then it is likely going to cause the corresponding kernel client driver crash.
-Sumit