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