[0/9] Rework SCMI initialization and probing sequence

  Hi,

under some configurations the SCMI core stack, which is now initialized
as a whole at the subsys_initcall level, can be dependent on some other
Kernel subsystems (like TEE) when some SCMI transport backend like optee
is used.

This has been reported to lead to some awkward probe loop which, even
though successful at the end, leaves a track of errors in the logs coming
directly from the core Linux driver model facilities.

In order to solve this issue and cleaning up a bit the SCMI stack startup
sequence, this small series reviews and reworks the SCMI core stack
initialization and probe logic.

Basically the SCMI Bus is split into its own module (scmi-core.ko) which is
initialized at subsys_initcall, while the SCMI core stack, including its
various transport backends (like optee, mailbox, virtio, smc), is kept into
a distinct module (scmi-module.ko) which get initialized at module_init.

The SCMI driver users initlevel, instead, remains unchanged at module_init.

No change is made to the Kconfig: the main ARM_SCMI_PROTOCOL option will
now cause both modules to be built.

This allows the other possibly needed subsystems to be up and running
well before the core SCMI stack and its dependent transport backends, so
solving the reported issue.

Tested with SCMI transports mailbox/virtio and, in a previous draft, optee,
in a number of different load/unload/bind/unbind combinations both as
builtin and as LKMs.

Applies on v6.1.

Any feedback, testing welcome.

Thanks,
Cristian

Cristian Marussi (9):
  firmware: arm_scmi: Simplify chan_available transport operation
  firmware: arm_scmi: Use dedicated devices to initialize channels
  firmware: arm_scmi: Move protocol registration helpers
  firmware: arm_scmi: Add common notifier helpers
  firmware: arm_scmi: Refactor protocol device creation
  firmware: arm_scmi: Move handle get/set helpers
  firmware: arm_scmi: Refactor device create/destroy helpers
  firmware: arm_scmi: Introduce a new lifecycle for protocol devices
  firmware: arm_scmi: Split bus and driver into distinct modules

 drivers/firmware/arm_scmi/Makefile  |   8 +-
 drivers/firmware/arm_scmi/bus.c     | 388 ++++++++++++-----
 drivers/firmware/arm_scmi/common.h  |  25 +-
 drivers/firmware/arm_scmi/driver.c  | 623 ++++++++++++++--------------
 drivers/firmware/arm_scmi/mailbox.c |   6 +-
 drivers/firmware/arm_scmi/optee.c   |   6 +-
 drivers/firmware/arm_scmi/smc.c     |   6 +-
 drivers/firmware/arm_scmi/virtio.c  |   4 +-
 include/linux/scmi_protocol.h       |   5 -
 9 files changed, 622 insertions(+), 449 deletions(-)
  

On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> Hi,
>
> under some configurations the SCMI core stack, which is now initialized
> as a whole at the subsys_initcall level, can be dependent on some other
> Kernel subsystems (like TEE) when some SCMI transport backend like optee
> is used.

Thanks Cristian for the rework, but this doesn't seem to address
reluctance to carry forward the DT legacy (see [1]).

TLDR, it has led to misrepresentation of OP-TEE transport as follows:

    First represented as a platform device via DT (compatible =
"linaro,scmi-optee";) and then
    Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)

Do we really need to have a platform device for every SCMI transport?

[1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/

-Sumit

>
> This has been reported to lead to some awkward probe loop which, even
> though successful at the end, leaves a track of errors in the logs coming
> directly from the core Linux driver model facilities.
>
> In order to solve this issue and cleaning up a bit the SCMI stack startup
> sequence, this small series reviews and reworks the SCMI core stack
> initialization and probe logic.
>
> Basically the SCMI Bus is split into its own module (scmi-core.ko) which is
> initialized at subsys_initcall, while the SCMI core stack, including its
> various transport backends (like optee, mailbox, virtio, smc), is kept into
> a distinct module (scmi-module.ko) which get initialized at module_init.
>
> The SCMI driver users initlevel, instead, remains unchanged at module_init.
>
> No change is made to the Kconfig: the main ARM_SCMI_PROTOCOL option will
> now cause both modules to be built.
>
> This allows the other possibly needed subsystems to be up and running
> well before the core SCMI stack and its dependent transport backends, so
> solving the reported issue.
>
> Tested with SCMI transports mailbox/virtio and, in a previous draft, optee,
> in a number of different load/unload/bind/unbind combinations both as
> builtin and as LKMs.
>
> Applies on v6.1.
>
> Any feedback, testing welcome.
>
> Thanks,
> Cristian
>
> Cristian Marussi (9):
>   firmware: arm_scmi: Simplify chan_available transport operation
>   firmware: arm_scmi: Use dedicated devices to initialize channels
>   firmware: arm_scmi: Move protocol registration helpers
>   firmware: arm_scmi: Add common notifier helpers
>   firmware: arm_scmi: Refactor protocol device creation
>   firmware: arm_scmi: Move handle get/set helpers
>   firmware: arm_scmi: Refactor device create/destroy helpers
>   firmware: arm_scmi: Introduce a new lifecycle for protocol devices
>   firmware: arm_scmi: Split bus and driver into distinct modules
>
>  drivers/firmware/arm_scmi/Makefile  |   8 +-
>  drivers/firmware/arm_scmi/bus.c     | 388 ++++++++++++-----
>  drivers/firmware/arm_scmi/common.h  |  25 +-
>  drivers/firmware/arm_scmi/driver.c  | 623 ++++++++++++++--------------
>  drivers/firmware/arm_scmi/mailbox.c |   6 +-
>  drivers/firmware/arm_scmi/optee.c   |   6 +-
>  drivers/firmware/arm_scmi/smc.c     |   6 +-
>  drivers/firmware/arm_scmi/virtio.c  |   4 +-
>  include/linux/scmi_protocol.h       |   5 -
>  9 files changed, 622 insertions(+), 449 deletions(-)
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
  

On Fri, Dec 23, 2022 at 11:06:29AM +0530, Sumit Garg wrote:
> On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
> >
> > Hi,
> >
> > under some configurations the SCMI core stack, which is now initialized
> > as a whole at the subsys_initcall level, can be dependent on some other
> > Kernel subsystems (like TEE) when some SCMI transport backend like optee
> > is used.
> 
> Thanks Cristian for the rework, but this doesn't seem to address
> reluctance to carry forward the DT legacy (see [1]).
> 
> TLDR, it has led to misrepresentation of OP-TEE transport as follows:
> 
>     First represented as a platform device via DT (compatible =
> "linaro,scmi-optee";) and then
>     Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
> 0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
> 
> Do we really need to have a platform device for every SCMI transport?
> 
> [1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
> 

Hi Sumit,

thanks for the feedback first of all.

This series represents really a long standing point on my todo-list and it
is meant to start addressing/reviewing the whole SCMI stack init/probe
sequencing and transports setup while taking the chance/opportunity to
fix the issue reported by Ludvig.

The natural next step in my (and Sudeep) view would be to split out the SCMI
transports too into proper full fledged drivers, that can be probed by their
own susbsys eventually (when possible) and that will then register with the
SCMI core as available transports; so that we can avoid some of the cruft
when multiple backend subsystems are involved...

...it is just that I have NOT dug deep into this further evolution and I did
NOT want to do it in this series, but just starting laying out some basic rework
toward this direction while fixing Ludvig issue. (... also because there are a
lot of bit and pieces to get right in SCMI around protocols/modules and DT
parsing and I was trying not to break too many things at a time :P...)

Anyway, even in the perspective of the above possible evolution into full
fledged drivers, I doubt that we can get rid completely of the DT based
per-transport platform devices since their DT nodes can carry a bit of
transport related information (even for auto-discoverable transport I think)

...it will just be that such devices, bound to the compatibles, will be used
probably in a different way (also for backward compatibility with DT
bindings...)...indeed...such platform devices now DO carry some information
about the underlying transport to use BUT most of all they represent also
an SCMI platform instance, so that will not definitely go away completely,
it will just loose most of the transport related functionalities

..but... as said...I have not dived too much into this further evolution so
I maybe wrong here on the details... anyway the plan going further, as spoken
also with Sudeep offline, could/should be that depicted above.

Not sure if this answers all of your questions but I'll keep you posted
on this series and next evolutions...

Thanks,
Cristian
  

On Fri, 23 Dec 2022 at 17:07, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> On Fri, Dec 23, 2022 at 11:06:29AM +0530, Sumit Garg wrote:
> > On Fri, 23 Dec 2022 at 00:22, Cristian Marussi <cristian.marussi@arm.com> wrote:
> > >
> > > Hi,
> > >
> > > under some configurations the SCMI core stack, which is now initialized
> > > as a whole at the subsys_initcall level, can be dependent on some other
> > > Kernel subsystems (like TEE) when some SCMI transport backend like optee
> > > is used.
> >
> > Thanks Cristian for the rework, but this doesn't seem to address
> > reluctance to carry forward the DT legacy (see [1]).
> >
> > TLDR, it has led to misrepresentation of OP-TEE transport as follows:
> >
> >     First represented as a platform device via DT (compatible =
> > "linaro,scmi-optee";) and then
> >     Migrated to being a TEE bus device (UUID: 0xa8cfe406, 0xd4f5,
> > 0x4a2e, 0x9f, 0x8d, 0xa2, 0x5d, 0xc7, 0x54, 0xc0, 0x99)
> >
> > Do we really need to have a platform device for every SCMI transport?
> >
> > [1] https://lore.kernel.org/lkml/CAFA6WYPwku8d7EiJ8rF5pVh568oy+jXMXLdxSr6r476e0SD2nw@mail.gmail.com/
> >
>
> Hi Sumit,
>
> thanks for the feedback first of all.
>
> This series represents really a long standing point on my todo-list and it
> is meant to start addressing/reviewing the whole SCMI stack init/probe
> sequencing and transports setup while taking the chance/opportunity to
> fix the issue reported by Ludvig.
>
> The natural next step in my (and Sudeep) view would be to split out the SCMI
> transports too into proper full fledged drivers, that can be probed by their
> own susbsys eventually (when possible) and that will then register with the
> SCMI core as available transports; so that we can avoid some of the cruft
> when multiple backend subsystems are involved...
>
> ...it is just that I have NOT dug deep into this further evolution and I did
> NOT want to do it in this series, but just starting laying out some basic rework
> toward this direction while fixing Ludvig issue. (... also because there are a
> lot of bit and pieces to get right in SCMI around protocols/modules and DT
> parsing and I was trying not to break too many things at a time :P...)
>
> Anyway, even in the perspective of the above possible evolution into full
> fledged drivers, I doubt that we can get rid completely of the DT based
> per-transport platform devices since their DT nodes can carry a bit of
> transport related information (even for auto-discoverable transport I think)
>
> ...it will just be that such devices, bound to the compatibles, will be used
> probably in a different way (also for backward compatibility with DT
> bindings...)...indeed...such platform devices now DO carry some information
> about the underlying transport to use BUT most of all they represent also
> an SCMI platform instance, so that will not definitely go away completely,
> it will just loose most of the transport related functionalities
>
> ..but... as said...I have not dived too much into this further evolution so
> I maybe wrong here on the details... anyway the plan going further, as spoken
> also with Sudeep offline, could/should be that depicted above.
>
> Not sure if this answers all of your questions but I'll keep you posted
> on this series and next evolutions...

Thanks for the detailed clarification. I don't have the deep insights
regarding how SCMI subsystem works but generally dealing with a device
being probed on multiple buses is prone to system integration problems
such as:

- Is the device present on the platform bus (in DT)? Is the device
present on a discoverable bus (eg. TEE bus)?
- Do both buses represent synchronised device views? IOW, version skew problems.

I hope we should be able to address those with the evolution you are planning.

-Sumit

>
> Thanks,
> Cristian
  

On Thu, 22 Dec 2022 18:50:40 +0000, Cristian Marussi wrote:
> under some configurations the SCMI core stack, which is now initialized
> as a whole at the subsys_initcall level, can be dependent on some other
> Kernel subsystems (like TEE) when some SCMI transport backend like optee
> is used.
> 
> This has been reported to lead to some awkward probe loop which, even
> though successful at the end, leaves a track of errors in the logs coming
> directly from the core Linux driver model facilities.
> 
> [...]

Applied to sudeep.holla/linux (for-next/scmi), thanks!

[1/9] firmware: arm_scmi: Simplify chan_available transport operation
      https://git.kernel.org/sudeep.holla/c/7a75b7afd8ff
[2/9] firmware: arm_scmi: Use dedicated devices to initialize channels
      https://git.kernel.org/sudeep.holla/c/05a2801d8b90
[3/9] firmware: arm_scmi: Move protocol registration helpers
      https://git.kernel.org/sudeep.holla/c/9115c20ac1ee
[4/9] firmware: arm_scmi: Add common notifier helpers
      https://git.kernel.org/sudeep.holla/c/53b8c25df708
[5/9] firmware: arm_scmi: Refactor protocol device creation
      https://git.kernel.org/sudeep.holla/c/d3cd7c525fd2
[6/9] firmware: arm_scmi: Move handle get/set helpers
      https://git.kernel.org/sudeep.holla/c/971fc0665f13
[7/9] firmware: arm_scmi: Refactor device create/destroy helpers
      https://git.kernel.org/sudeep.holla/c/2c3e674465e7
[8/9] firmware: arm_scmi: Introduce a new lifecycle for protocol devices
      https://git.kernel.org/sudeep.holla/c/ee5dcedaf72d
[9/9] firmware: arm_scmi: Split bus and driver into distinct modules
      https://git.kernel.org/sudeep.holla/c/37b5be828032
--
Regards,
Sudeep