On Mon, Nov 30, 2020 at 08:27:54PM +0200, Andy Shevchenko wrote:
> On Mon, Nov 30, 2020 at 5:42 PM Johan Hovold <johan@xxxxxxxxxx> wrote:
> >
> > Add a nordy sysfs attribute to suppress raising the modem-control lines
> > on open to signal DTE readiness.
>
> Why not call it nomctrl ?
That was one of the candidates I rejected.
As I hinted in the cover letter (and patch adding the flag) I chose the
name to match the current termios flags (e.g. HUPCL and NOFLSH).
NOMCTRL is both too general and specific; HUPCL still controls the
modem-control lines on final close. Also, like HUPCL, I wanted a more
general name that can be used for terminal devices which can signal
readiness through other means (e.g. network).
Like the other termios flags it is terse, but once you learn the meaning
it's easy to remember. And I think there's value in keeping the same
name throughout (cf. termios flags and stty).
> > This can be use to prevent undesirable side-effects on open for
>
> used
Thanks, I'll fix that up before applying or resending
> > applications where the DTR and RTS lines are used for non-standard
> > purposes such as generating power-on and reset pulses.
>
> ...
>
> > +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);
> > + unsigned int val;
> > + int ret;
> > +
> > + ret = kstrtouint(buf, 0, &val);
> > + if (ret)
> > + return ret;
>
> > + if (val > 1)
> > + return -EINVAL;
>
> Can't we utilise kstrtobool() instead?
I chose not to as kstrtobool() results in a horrid interface. To many
options to do the same thing and you end up with confusing things like
"0x01" being accepted but treated as false (as only the first character
is considered).
Not sure how that ever made it into sysfs code...
The attribute is read back as "0" or "1" and those are precisely the
values that can be written back (well, modulo radix).
It's not relevant in this case, but tight control over the inputs also
allows for extending the range later.
> > + tty_port_set_nordy(port, val);
> > +
> > + return count;
> > +}
Johan
