lkml.org 
[lkml]   [2015]   [Mar]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/3] TTY: add support for tty_slave devices.
    Hi,

    On 03/18/2015, 06:58 AM, NeilBrown wrote:
    > --- /dev/null
    > +++ b/drivers/tty/slave/tty_slave_core.c
    > @@ -0,0 +1,136 @@

    ...

    > +static int tty_slave_match(struct device *dev, struct device_driver *drv)
    > +{
    > + return of_driver_match_device(dev, drv);
    > +}
    > +
    > +static void tty_slave_release(struct device *dev)
    > +{
    > + kfree(dev);

    This should free the slave where the dev is contained. This is never
    called IMO due to missing put_device's in the code.

    > +}
    > +
    > +struct bus_type tty_slave_bus_type = {
    > + .name = "tty-slave",
    > + .match = tty_slave_match,
    > +};
    > +
    > +int tty_slave_register(struct device *parent, struct device_node *node,
    > + struct device *tty, struct tty_driver *drv)
    > +{
    > + struct tty_slave *slave;
    > + int retval;
    > +
    > + if (!of_get_property(node, "compatible", NULL))
    > + return -ENODEV;
    > +
    > + slave = kzalloc(sizeof(*slave), GFP_KERNEL);
    > + if (!slave)
    > + return -ENOMEM;
    > +

    device_initialize();

    > + slave->dev.bus = &tty_slave_bus_type;
    > + slave->dev.parent = parent;
    > + slave->dev.release = tty_slave_release;
    > + slave->dev.of_node = of_node_get(node);
    > + dev_set_name(&slave->dev, "%s", node->name);
    > + slave->tty_dev = tty;
    > + slave->tty_drv = drv;
    > + slave->ops = *drv->ops;
    > + retval = device_register(&slave->dev);
    > + if (retval) {
    > + of_node_put(node);
    > + kfree(slave);

    Do device_put() instead of the two. And do the two in .release.

    > + }
    > + return retval;
    > +}
    > +EXPORT_SYMBOL(tty_slave_register);
    ...
    > --- a/drivers/tty/tty_io.c
    > +++ b/drivers/tty/tty_io.c
    ...
    > @@ -3205,6 +3208,29 @@ static void tty_device_create_release(struct device *dev)
    > kfree(dev);
    > }
    >
    > +int tty_register_finalize(struct tty_driver *driver, struct device *dev)
    > +{
    > + int retval;
    > + bool cdev = false;
    > + int index = dev->devt - MKDEV(driver->major,
    > + driver->minor_start);
    > + printk("REGISTER %d %d 0x%x %d\n", driver->major, driver->minor_start, dev->devt, index);
    > + if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC)) {
    > + retval = tty_cdev_add(driver,
    > + dev->devt,
    > + index, 1);
    > + if (retval)
    > + return retval;
    > + cdev = true;
    > + }
    > + retval = device_register(dev);
    > + if (retval == 0)
    > + return 0;
    > + if (cdev)
    > + cdev_del(&driver->cdevs[index]);
    > + return retval;
    > +}
    > +EXPORT_SYMBOL(tty_register_finalize);
    > /**
    > * tty_register_device_attr - register a tty device
    > * @driver: the tty driver that describes the tty device
    > @@ -3234,7 +3260,8 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
    > dev_t devt = MKDEV(driver->major, driver->minor_start) + index;
    > struct device *dev = NULL;
    > int retval = -ENODEV;
    > - bool cdev = false;
    > + struct device_node *node;
    > + bool slave_registered = false;
    >
    > if (index >= driver->num) {
    > printk(KERN_ERR "Attempt to register invalid tty line number "
    > @@ -3247,13 +3274,6 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
    > else
    > tty_line_name(driver, index, name);
    >
    > - if (!(driver->flags & TTY_DRIVER_DYNAMIC_ALLOC)) {
    > - retval = tty_cdev_add(driver, devt, index, 1);
    > - if (retval)
    > - goto error;
    > - cdev = true;
    > - }
    > -
    > dev = kzalloc(sizeof(*dev), GFP_KERNEL);
    > if (!dev) {
    > retval = -ENOMEM;
    > @@ -3268,16 +3288,24 @@ struct device *tty_register_device_attr(struct tty_driver *driver,
    > dev->groups = attr_grp;
    > dev_set_drvdata(dev, drvdata);
    >
    > - retval = device_register(dev);
    > - if (retval)
    > - goto error;
    > + if (device && device->of_node)
    > + for_each_available_child_of_node(device->of_node, node) {
    > + if (tty_slave_register(device, node, dev, driver) == 0)
    > + slave_registered = true;
    > + if (slave_registered)
    > + break;
    > + }
    > +
    > + if (!slave_registered) {
    > + retval = tty_register_finalize(driver, dev);
    > + if (retval)
    > + goto error;
    > + }
    >
    > return dev;

    And what about ttys not using the tty_register_device* helpers?

    What happens when the tty is unregistered?

    > --- /dev/null
    > +++ b/include/linux/tty_slave.h
    > @@ -0,0 +1,26 @@
    > +
    > +struct tty_slave {
    > + struct device *tty_dev;
    > + struct tty_driver *tty_drv;
    > + struct tty_operations ops;
    > + struct device dev;
    > +};
    > +
    > +int tty_slave_finalize(struct tty_slave *slave);
    > +int tty_slave_driver_register(struct device_driver *drv);
    > +#if config_enabled(CONFIG_TTY_SLAVE)

    Since TTY_SLAVE is tristate, this will work only for TTY_SLAVE == y, not
    m. Use IS_ENABLED instead.

    > +void tty_slave_activate(struct tty_struct *tty);
    > +int tty_slave_register(struct device *parent, struct device_node *node,
    > + struct device *tty, struct tty_driver *drv);
    > +#else
    > +static inline void tty_slave_activate(struct tty_struct *tty)
    > +{
    > +}
    > +static inline int tty_slave_register(struct device *parent,
    > + struct device_node *node,
    > + struct device *tty,
    > + struct tty_driver *drv)
    > +{
    > + return -ENODEV;
    > +}
    > +#endif

    thanks,
    --
    js
    suse labs


    \
     
     \ /
      Last update: 2015-03-24 11:41    [W:2.566 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site