Linux-Serial Archive mirror
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>,
	Rob Herring <robh@kernel.org>, Jiri Slaby <jirislaby@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Serial <linux-serial@vger.kernel.org>,
	Elvis <elvisimprsntr@gmail.com>
Subject: Re: Fwd: Add method to allow switching kernel level PPS signal from DCD to CTS serial pin
Date: Thu, 9 May 2024 02:24:56 -0400	[thread overview]
Message-ID: <20240509062456.GE3620298@mit.edu> (raw)
In-Reply-To: <2024050853-basin-salsa-32bb@gregkh>

On Wed, May 08, 2024 at 07:28:35PM +0100, Greg Kroah-Hartman wrote:
> On Wed, May 08, 2024 at 08:52:24AM +0700, Bagas Sanjaya wrote:
> > What do you think about above feature request?
> 
> We will be glad to review any submitted patches for any features.
> patches in bugzilla are not viable for obvious reasons.

Bagas,

Note that the feature request is related to PPS, so per the
MAINTAINERS file entry:

PPS SUPPORT
M:	Rodolfo Giometti <giometti@enneenne.com>
L:	linuxpps@ml.enneenne.com (subscribers-only)
S:	Maintained
W:	http://wiki.enneenne.com/index.php/LinuxPPS_support
F:	Documentation/ABI/testing/sysfs-pps
F:	Documentation/devicetree/bindings/pps/pps-gpio.yaml
F:	Documentation/driver-api/pps.rst
F:	drivers/pps/
F:	include/linux/pps*.h
F:	include/uapi/linux/pps.h

I'd suggest that you reach out to Rondolfo as the maintainer, or to
the linuxpps mailing list.

First of all, looking at the patch referenced in the bugzilla (which
is actually found in github), it appears that the person who made the
request via Bugzilla is different from the the person who authored the
patch (apparently, github.com/not1337).

Secondly, the patch is really quite hacky.  First, the termonology
used of "4wire" is non-standard (e.g., uised nowhere but at
github.com/not1337/pss-stuff), and misleading.  A cable which only has
RxD, TxD, RTS, and CTS is not going to work well without GND, so "4
wire" is quite the misnomer".  This termonology is also not used by
FreeBSD, BTW.  Secondly, unconditionally mapping CTS to DCD when
setting a magic UART-level attribute is a bit hacky, since it will do
this magic ad-hoc mapping all of the time, not only if the PPS line
discpline is selected.

Now, I haven't been the tty maintainer in quite a while, but in my
opinion, a much cleaner way would be to plumb a new tty ldisc
function, cts_change, which is analogous to the dcd_change function
(which was introduced specifically for pps_ldisc).  Then for bonus
points, consider using the pps capture mode mde that FreeeBSD's UART
driver, including the invert option and narrow pulse mode, and eschew
using the non-standard "4wire" naming terminology.

Finally, note that the way kernel development works is that it's not
enough for a user to ask for a feature.  Someone has to create a high
quality, clean, maintainable patch.  Note all random hacks found in
random Bugzilla or Github git trees are suitable for inclusion in the
upstream kernel.  And if you don't know how to evaluate the patch for
quality, it might not be best thing to just ask the bugzilla requester
to follow the Submitting Patches procedure, given that (a) they might
not be a kernel developer, and (b) it might just frustrate the
bugzilla requester and maintainer if the patch isn't sufficient high
quality, especially if you've managed to set expectations that all the
bugzilla requestor needs to do is to submit the patch and it will be
accepted.

Cheers,

					- Ted

  reply	other threads:[~2024-05-09  6:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-08  1:52 Fwd: Add method to allow switching kernel level PPS signal from DCD to CTS serial pin Bagas Sanjaya
2024-05-08 18:28 ` Greg Kroah-Hartman
2024-05-09  6:24   ` Theodore Ts'o [this message]
2024-05-09 10:41     ` Bagas Sanjaya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240509062456.GE3620298@mit.edu \
    --to=tytso@mit.edu \
    --cc=bagasdotme@gmail.com \
    --cc=elvisimprsntr@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).