Re: [PATCH 09/10] serdev: add serdev_device_set_rts

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

 



Sebastian Reichel, on lun. 20 mars 2017 03:45:25 +0100, wrote:
> On Thu, Mar 16, 2017 at 04:52:26PM +0100, Samuel Thibault wrote:
> > Rob Herring, on jeu. 16 mars 2017 09:02:07 -0500, wrote:
> > > On Thu, Mar 16, 2017 at 4:07 AM, Samuel Thibault
> > > <samuel.thibault@xxxxxxxxxxxx> wrote:
> > > > Sebastian Reichel, on jeu. 16 mars 2017 09:02:40 +0000, wrote:
> > > >> +void serdev_device_set_rts(struct serdev_device *serdev, bool enable)
> > > >
> > > > Rather than defining a set_rts(enable), and then someone will want to
> > > > set DTR, and similarly, get_cts() + get_dsr()... Why not sticking to the
> > > > TTY ops, i.e. defining serdev_device_tiocmset(serdev, set, clear) and
> > > > serdev_device_tiocmget? That'd seem much more straightforward to me and
> > > > coherent with the TTY ops.
> > > 
> > > I'd prefer to keep operations as specific functions that we can make
> > > slightly higher level and common across drivers. For example, a "wait
> > > for CTS" function rather than drivers implementing their own wait
> > > loops.
> > 
> > For waiting, I completely agree, but for setting, it looks odd not to
> > use the tiocmset(set,clear) interface as implemented by TTY, which is as
> > convenient to use as set_rts(enable) while being universal.
> 
> How about implementing serdev_device_set_tiocm and provide a
> few helpers:
> 
> static inline void serdev_device_set_rts(serdev, enable) {
>     if (enable)
>         return serdev_device_set_tiocm(serdev, RTS, 0);
>     else
>         return serdev_device_set_tiocm(serdev, 0, RTS);
> }

Why not indeed.  My point is just that tiocmset (why using yet another
name) is useful when plugging into yet another serial device system,
which is way simpler with just one function than the series.

Samuel
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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