Re: [PATCH] fix serdev bind/unbind

[Rob Herring]

+Johan

On Tue, Jan 18, 2022 at 1:47 PM julian schroeder
<julianmarcusschroeder@xxxxxxxxx> wrote:
>
> On some chromebooks, the serdev is used to communicate with
> an embedded controller. When the controller is updated, the
> regular ttyS* is needed. Therefore unbind/bind needs to work
> to be able to switch between the two modes without having to
> reboot. In the case of ACPI enabled platforms, the underlying
> serial device is marked as enumerated but this is not cleared
> upon remove (unbind). In this state it can not be bound as
> serdev.

'fix' implies this was supposed to work and doesn't, but unbind/bind
was never a feature of serdev. Or more specifically, switching between
serdev and tty was not a feature. There have been some attempts to add
that. I suspect it is more than a 4 line change based on those, but
maybe I'm wrong.

For your usecase, how does a given piece of h/w that needs and/or
provides kernel support continue to work when the driver is unbound.
Are you leaving any power controls that the serdev driver configured
enabled so that the tty happens to keep working? What happens to
interfaces the EC provides? The kernel doesn't deal with resources
going away too well. I have to wonder if the existing serdev EC driver
should learn to handle the 'update mode' itself or provide some sort
of raw/passthru mode to userspace. A TTY, while standard, brings a lot
of complexities.

> Signed-off-by: julian schroeder <julianmarcusschroeder@xxxxxxxxx>
> ---
>  drivers/tty/serdev/core.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 92e3433276f8..668fa570bc07 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -138,7 +138,11 @@ EXPORT_SYMBOL_GPL(serdev_device_add);
>  void serdev_device_remove(struct serdev_device *serdev)
>  {
>         struct serdev_controller *ctrl = serdev->ctrl;
> +       struct acpi_device *adev;
>
> +       adev = ACPI_COMPANION(&serdev->dev);
> +       if (adev)
> +               acpi_device_clear_enumerated(adev);
>         device_unregister(&serdev->dev);
>         ctrl->serdev = NULL;
>  }
> --
> 2.20.1
>