lkml.org 
[lkml]   [2018]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH v2 00/19] Dynamically load/remove serdev devices via sysfs*
Hi Johan
On Thu, Jun 14, 2018 at 12:48 PM Johan Hovold <johan@kernel.org> wrote:
>
> Hi Ricardo,
>
> On Mon, Jun 11, 2018 at 01:52:16PM +0200, Ricardo Ribalda Delgado wrote:
> > There are some situations where it is interesting to load/remove serdev
> > devices dynamically, like during board bring-up or when we are
> > developing a new driver or for devices that are neither described via
> > ACPI or device tree.
>
> First of all, this would be more appropriately labeled an RFC as this is
> far from being in a mergeable state. Besides some implementation issues,
> we need to determine if this approach is at all viable.

From previous conversations with Greg it seemed that RFC labels was
something to avoid, but I do not mind reseding it as RFC on v3.

http://lists.kernelnewbies.org/pipermail/kernelnewbies/2018-March/018844.html

>
> Second, I wonder how you tested this given that this series breaks RX
> (and write-wakeup signalling) for serial ports (patch 19/24)?

I have a serdev device (led ctrls) that is dynamically enumerated with
something very similar to:

https://github.com/ribalda/linux/commit/415bb3f0076c2b846ebe5409589b8e1e3004f55a

and then I have a script that does adds and removes. For standard
serial port I was not testing the data path, just that the ttyS* was
enumerated fine.

But yesterday I believe that we found the bug that you mentioned and
we have fixed it (check end of mail). I will patch the series and
resend after I get more feedback and also implement what Marcel
suggested.

WIP is at
https://github.com/ribalda/linux/tree/serdev3

Besides this bug, we have used the new driver for over a week now with
no issues.

>
> Third, and as Marcel already suggested, you need to limit your scope
> here. Aim at ten patches or so, and use a representative serdev driver
> as an example of the kind of driver updates that would be needed. It
> also looks like some patches should be squashed (e.g. the ones
> introducing new fields and the first one actually using them).

>
> > This implementation allows the creation of serdev devices via sysfs,
> > in a similar way as the i2c bus allows sysfs instantiation [1].
>
> Note that this is a legacy interface and not necessarily something that
> new interfaces should be modelled after.

I would not consider it legacy, it is the only way to use an i2c
module without writing your own driver and/or modifying ACPI/DT table.
Just google around for i2c linux...
Thanks!

>
> Johan
Author: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
Date: Thu Jun 14 11:30:27 2018 +0200

serdev-ttydev: Restore/change ttyport client ops

diff --git a/drivers/tty/serdev/serdev-ttydev.c
b/drivers/tty/serdev/serdev-ttydev.c
index 180035e101dc..b151c9645a1d 100644
--- a/drivers/tty/serdev/serdev-ttydev.c
+++ b/drivers/tty/serdev/serdev-ttydev.c
@@ -16,14 +16,23 @@ static int ttydev_serdev_probe(struct serdev_device *serdev)
struct serdev_controller *ctrl = serdev->ctrl;
struct serport *serport;
struct device *dev;
+ const struct tty_port_client_operations *serdev_ops;

if (!ctrl->is_ttyport)
return -ENODEV;

serport = serdev_controller_get_drvdata(ctrl);

+ serdev_ops = serport->port->client_ops;
+
+ serport->port->client_ops = serport->tty_ops;
dev = tty_register_device_attr(serport->tty_drv, serport->tty_idx,
- &serdev->dev, NULL, NULL);
+ ctrl->dev.parent, NULL, NULL);
+
+ if (IS_ERR(dev))
+ serport->port->client_ops = serdev_ops;
+ else
+ serdev_device_set_drvdata(serdev, (void *)serdev_ops);

return dev ? 0 : PTR_ERR(dev);
}
@@ -35,6 +44,7 @@ static void ttydev_serdev_remove(struct serdev_device *serdev)

serport = serdev_controller_get_drvdata(ctrl);
tty_unregister_device(serport->tty_drv, serport->tty_idx);
+ serport->port->client_ops = serdev_device_get_drvdata(serdev);
}


--
Ricardo Ribalda

\
 
 \ /
  Last update: 2018-06-14 13:08    [W:0.349 / U:0.588 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site