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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux