Message-ID: <CAGptzHOOqLhBnAXDURAzkgckUvRr__UuF1S_7MLV0u-ZxYEdyA@mail.gmail.com>
Date: Tue, 16 Dec 2025 13:08:38 +0530
From: Sumit Garg <sumit.garg@....qualcomm.com>
To: Uwe Kleine-König <u.kleine-koenig@...libre.com>
Cc: Sumit Garg <sumit.garg@...nel.org>,
        Jens Wiklander <jens.wiklander@...aro.org>,
        Olivia Mackall <olivia@...enic.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Clément Léger <clement.leger@...tlin.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Ard Biesheuvel <ardb@...nel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Jan Kiszka <jan.kiszka@...mens.com>,
        Sudeep Holla <sudeep.holla@....com>,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>,
        Michael Chan <michael.chan@...adcom.com>,
        Pavan Chebbi <pavan.chebbi@...adcom.com>,
        Rafał Miłecki <zajec5@...il.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        Jarkko Sakkinen <jarkko@...nel.org>, Mimi Zohar <zohar@...ux.ibm.com>,
        David Howells <dhowells@...hat.com>, Paul Moore <paul@...l-moore.com>,
        James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
        Peter Huewe <peterhuewe@....de>, op-tee@...ts.trustedfirmware.org,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-rtc@...r.kernel.org, linux-efi@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm-kernel@...ts.infradead.org,
        Cristian Marussi <cristian.marussi@....com>, arm-scmi@...r.kernel.org,
        netdev@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-integrity@...r.kernel.org, keyrings@...r.kernel.org,
        linux-security-module@...r.kernel.org, Jason Gunthorpe <jgg@...pe.ca>
Subject: Re: [PATCH v1 00/17] tee: Use bus callbacks instead of driver callbacks

Hi Uwe,

On Mon, Dec 15, 2025 at 3:02 PM Uwe Kleine-König
<u.kleine-koenig@...libre.com> wrote:
>
> Hello Sumit,
>
> On Mon, Dec 15, 2025 at 04:54:11PM +0900, Sumit Garg wrote:
> > On Thu, Dec 11, 2025 at 06:14:54PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > the objective of this series is to make tee driver stop using callbacks
> > > in struct device_driver. These were superseded by bus methods in 2006
> > > (commit 594c8281f905 ("[PATCH] Add bus_type probe, remove, shutdown
> > > methods.")) but nobody cared to convert all subsystems accordingly.
> > >
> > > Here the tee drivers are converted. The first commit is somewhat
> > > unrelated, but simplifies the conversion (and the drivers). It
> > > introduces driver registration helpers that care about setting the bus
> > > and owner. (The latter is missing in all drivers, so by using these
> > > helpers the drivers become more correct.)
> > >
> > > The patches #4 - #17 depend on the first two, so if they should be
> > > applied to their respective subsystem trees these must contain the first
> > > two patches first.
> >
> > Thanks Uwe for your efforts to clean up the boilerplate code for TEE bus
> > drivers.
>
> Thanks for your feedback. I will prepare a v2 and address your comments
> (whitespace issues and wrong callback in the shutdown method).
>
> > > Note that after patch #2 is applied, unconverted drivers provoke a
> > > warning in driver_register(), so it would be good for the user
> > > experience if the whole series goes in during a single merge window.
> >
> > +1
> >
> > I suggest the whole series goes via the Jens tree since there shouldn't
> > be any chances for conflict here.
> >
> > > So
> > > I guess an immutable branch containing the frist three patches that can
> > > be merged into the other subsystem trees would be sensible.
> > >
> > > After all patches are applied, tee_bus_type can be made private to
> > > drivers/tee as it's not used in other places any more.
> > >
> >
> > 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.

> 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.

>
> 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.

>
>  - 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.

-Sumit

Powered by blists - more mailing lists