Linux USB
 help / color / mirror / Atom feed
From: Oliver Neukum <oneukum@suse.com>
To: Simon Arlott <simon@octiron.net>,
	Oliver Neukum <oneukum@suse.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-serial@vger.kernel.org
Subject: Re: [PATCH] USB: cdc-acm: expose serial close_delay and closing_wait in sysfs
Date: Thu, 24 Aug 2023 10:21:19 +0200	[thread overview]
Message-ID: <6db3bf2b-b284-7e17-8c42-582a5c7addef@suse.com> (raw)
In-Reply-To: <ea1a13ad-a1e0-540a-e97a-4c44f6d2d33b@0882a8b5-c6c3-11e9-b005-00805fc181fe.uuid.home.arpa>

On 23.08.23 22:37, Simon Arlott wrote:

Hi,

I am terribly sorry, but this has to be a hard NO.
This patch has an unsurmountable problem.
Reasons below.
  
> The serial_core driver (ttySx) exposes its supported ioctl values as
> read-only sysfs attributes. Add read-write sysfs attributes "close_delay"
> and "closing_wait" to cdc-acm (ttyACMx) devices. These are the same as the

If the tty core does not export them as writable it presumably has reasons
for that. We cannot circumvent those reasons in a particular driver unless
there are very special circumstances that make the driver a special case.

The correct way to deal with this issue would be a proposal to change
the generic serial code. Even there I am sceptical because we would carry
the code duplication forever. ioctl() is the way you set the parameters
for a serial port in a Unix system. We have tools for that. Adding a second method
is problematic.
But that is not for me to decide. As far as CDC-ACM, however, is concerned,
I must reject this patch, because it fundamentally does something that should
not be done, definitely not at this layer.

	Sorry
		Oliver
  
> Signed-off-by: Simon Arlott <simon@octiron.net>
Nacked-by: Oliver Neukum <oneukum@suse.com>

  parent reply	other threads:[~2023-08-24  8:22 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-23 20:37 [PATCH] USB: cdc-acm: expose serial close_delay and closing_wait in sysfs Simon Arlott
2023-08-23 21:12 ` [PATCH (v2)] " Simon Arlott
2023-08-24  7:18   ` [PATCH] docs: ABI: sysfs-tty: close times are in hundredths of a second Simon Arlott
2023-08-25  5:33     ` Jiri Slaby
2023-08-27 18:23       ` [PATCH (v2)] docs: ABI: sysfs-tty: close times are in centiseconds Simon Arlott
2023-08-24  8:21 ` Oliver Neukum [this message]
2023-08-24 14:48 ` [PATCH] USB: cdc-acm: expose serial close_delay and closing_wait in sysfs Greg Kroah-Hartman
2023-08-24 18:02   ` Simon Arlott
2023-08-24 19:22     ` Greg Kroah-Hartman
2023-08-27 15:36       ` Simon Arlott
2023-08-24 23:46     ` Oliver Neukum
2023-08-25  1:53       ` Alan Stern

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=6db3bf2b-b284-7e17-8c42-582a5c7addef@suse.com \
    --to=oneukum@suse.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=simon@octiron.net \
    /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