Hi,
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);
}
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
