Hello Amit,
On Fri, Feb 14, 2025 at 11:58:44AM +0100, Amit Shah wrote:
> On Thu, 2025-02-13 at 12:55 +0100, Uwe Kleine-König wrote:
> > virtio_console.c can make use of REMOTEPROC. Therefore it has several
> > tests evaluating
> >
> > IS_ENABLED(CONFIG_REMOTEPROC)
> >
> > . This currently only does the right thing because CONFIG_REMOTEPROC
> > cannot be modular. Otherwise the configuration
> >
> > CONFIG_REMOTEPROC=m
> > CONFIG_VIRTIO_CONSOLE=y
> >
> > would result in a build failure because then
> > IS_ENABLED(CONFIG_REMOTEPROC) evaluates to true but still the built-
> > in
> > virtio_console.o must not use symbols from the remoteproc module.
> >
> > To prepare for making REMOTEPROC modular change the tests to use
> > IS_REACHABLE() instead of IS_ENABLED() which copes correctly for the
> > above case as it evaluates to false then.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> > ---
> > Hello,
> >
> > I didn't check what else needs to be done to make CONFIG_REMOTEPROC
> > tristate but even if it stays a bool using IS_REACHABLE() is still
> > the
> > better choice.
>
> It might lead to a false sense of "better" -- the value of IS_ENABLED
> is cached in a variable which is determined at compile-time.
Either I don't understand what you mean, or this is wrong.
$ make allmodconfig drivers/char/virtio_console.i
$ grep CONFIG_REMOTEPROC= .config
CONFIG_REMOTEPROC=m
$ cat drivers/char/virtio_console.i
...
static bool is_rproc_serial(const struct virtio_device *vdev)
{
return 1 && vdev->id.device == 11;
}
...
so is_rproc_enabled is still constant and known at compile time.
> That
> caching, after this change, moves to driver init-time. If the rproc
> module is loaded after virtio-console is initialized, there's no way
> it's going to be used.
If both are modular, modprobe should make sure that rproc is ready
before virtio-console. If virtio-console is builtin and rproc is
modular, IS_REACHABLE(CONFIG_REMOTEPROC) evaluates to false and so rproc
won't be used. (As is the case already today with CONFIG_REMOTEPROC=n).
> Only if the rproc module is loaded before
> virtio-console will the rproc functionality be used -- which means that
> nothing changed in reality..
With that patch indeed nothing changed yet, because CONFIG_REMOTEPROC
cannot be =m today. Until this changes IS_REACHABLE() and IS_ENABLED()
are equivalent.
> To properly detect and use rproc if available would need the rproc
> initialization out of virtcons_probe() and into something that happens
> either via sysfs for existing ports, or when adding a new port to a
> device. However, the current spec doesn't allow for that, so some more
> changes will need to be made to ensure current backwards compat, and a
> new specification that allows for a late init of rproc.
I didn't understand that and hope it's irrelevant with the things I
wrote above.
Best regards
Uwe
Attachment:
signature.asc
Description: PGP signature
