Re: [PATCH v2 2/6] serial: core: add sysfs attribute to suppress ready signalling on open

Mychaela Falconia
2022-06-02T17:03:14Z

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~