| Message ID | 20220531043655.DDF783740232@freecalypso.org |
|---|---|
| State | New |
| Headers | show |
| Series | serial ports: add ability to suppress raising DTR & RTS on open | expand |
Jiri Slaby wrote: > As long as you break no currently supported devices. This condition is a given, and has always been satisfied in all of my patch proposals, including those patches which I currently instruct my users to install locally to work with my GSM devices. > I've just noticed the double negative "!tty_port_nordy()" on both calls of > that function. I guess there was already a discussion about the naming, > but wouldn't it make more sense to dub it like tty_port_do_rtscts()? There are two aspects to this naming issue: 1) names of internal flags and functions that exist only inside the kernel; 2) name of the sysfs attribute exported to userspace. The latter part constitutes ABI, hence it is the one that calls for serious reflection on the choice of name. The sysfs portion of the present patch series (as opposed to the USB VID:PID-keyed portion) originates from Johan, and the nordy name for the sysfs attribute (and for the internal flag and function for consistency) was his choice. My own preferred choice for the sysfs attribute name would be something like manual_rtsdtr rather than nordy; I feel that a name such as manual_rtsdtr conveys what is being done: asking the kernel to put these modem control outputs under manual control (TIOCMBIS & TIOCMBIC) instead of automatic assertion on open. Johan argued a year and a half ago that nordy was a better name as it indicated "please suppress ready signaling" - it's a different perspective. Johan, are you still around? Do you still favor nordy as the sysfs attribute name? At the end of the day, I will be happy with _any_ name for the sysfs attribute - to end users it's the functionality that matters, not the name - and because it's ABI, once the sysfs attribute is implemented with some given name, it will stay. So, can some authoritative people please weigh in on how this sysfs attribute should be named? Should it be nordy? manual_rtsdtr? Something else? M~
diff --git a/Documentation/ABI/testing/sysfs-tty b/Documentation/ABI/testing/sysfs-tty index 820e412d38a8..98cb5cf0af75 100644 --- a/Documentation/ABI/testing/sysfs-tty +++ b/Documentation/ABI/testing/sysfs-tty @@ -161,3 +161,10 @@ Contact: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Description: Allows user to detach or attach back the given device as kernel console. It shows and accepts a boolean variable. + +What: /sys/class/tty/ttyS0/nordy +Date: November 2020 +Contact: Johan Hovold <johan@kernel.org> +Description: + Show and store the port NORDY flag which suppresses raising + the modem-control lines on open to signal DTE readiness. diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 9a85b41caa0a..a17ac4efaceb 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2870,6 +2870,30 @@ static ssize_t console_store(struct device *dev, return ret < 0 ? ret : count; } +static ssize_t nordy_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct tty_port *port = dev_get_drvdata(dev); + + return sprintf(buf, "%d\n", tty_port_nordy(port)); +} + +static ssize_t nordy_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct tty_port *port = dev_get_drvdata(dev); + bool val; + int ret; + + ret = kstrtobool(buf, &val); + if (ret) + return ret; + + tty_port_set_nordy(port, val); + + return count; +} + static DEVICE_ATTR_RO(uartclk); static DEVICE_ATTR_RO(type); static DEVICE_ATTR_RO(line); @@ -2884,6 +2908,7 @@ static DEVICE_ATTR_RO(io_type); static DEVICE_ATTR_RO(iomem_base); static DEVICE_ATTR_RO(iomem_reg_shift); static DEVICE_ATTR_RW(console); +static DEVICE_ATTR_RW(nordy); static struct attribute *tty_dev_attrs[] = { &dev_attr_uartclk.attr, @@ -2900,6 +2925,7 @@ static struct attribute *tty_dev_attrs[] = { &dev_attr_iomem_base.attr, &dev_attr_iomem_reg_shift.attr, &dev_attr_console.attr, + &dev_attr_nordy.attr, NULL };