On 10/30/2014 08:08 AM, Johan Hovold wrote:
> Hi Peter,
>
> Forgot to reply to this one.
>
> On Wed, Oct 22, 2014 at 07:40:20AM -0400, Peter Hurley wrote:
>> On 10/19/2014 01:12 PM, Johan Hovold wrote:
>>> [ +CC: Jiri, Alan, linux-serial ]
>>>
>>> On Thu, Oct 16, 2014 at 02:09:29PM -0400, Peter Hurley wrote:
>>>> On 10/16/2014 01:59 PM, Peter Hurley wrote:
>
>>>>> @@ -541,10 +531,6 @@ static int kobil_ioctl(struct tty_struct *tty,
>>>>>
>>>>> switch (cmd) {
>>>>> case TCFLSH:
>>>>> - transfer_buffer = kmalloc(transfer_buffer_length, GFP_KERNEL);
>>>>> - if (!transfer_buffer)
>>>>> - return -ENOBUFS;
>>>>> -
>>>>> result = usb_control_msg(port->serial->dev,
>>>>> usb_sndctrlpipe(port->serial->dev, 0),
>>>>> SUSBCRequest_Misc,
>>>>> @@ -559,7 +545,6 @@ static int kobil_ioctl(struct tty_struct *tty,
>>>>> dev_dbg(&port->dev,
>>>>> "%s - Send reset_all_queues (FLUSH) URB returns: %i\n",
>>>>> __func__, result);
>>>>> - kfree(transfer_buffer);
>>>>> return (result < 0) ? -EIO: 0;
>>>> ^^^
>>>> Returning 0 is almost certainly wrong; no further processing for
>>>> TCFLSH is performed.
>>>
>>> Indeed.
>>>
>>>> Only this driver returns 0 (of all the tty drivers in mainline).
>>>>
>>>> Returning -ENOIOCTLCMD allows further processing to continue;
>>>> especially the line discipline's input flushing, if TCIFLUSH/TCIOFLUSH.
>>>
>>> That doesn't seem like a very good idea, and only two *staging* drivers
>>> try to play such games (i.e. pretending not to implement the ioctl) as
>>> far as I can see.
>>
>> Well, returning EIONOCTLCMD is the standard method of ioctl passthrough
>> from driver to line discipline.
>
> I disagree with you there. AFAICS only these two staging drivers are
> abusing the meaning of EIONOCTLCMD (unrecognised ioctl) to have the
> line discipline also act on the ioctl.
Sorry, I wasn't as clear as I should have been here.
My point was that every driver gets ioctl(TCFLSH) and returns ENOIOCTLCMD
so that the line discipline will handle it. You're absolutely correct, in
that, only these drivers (and ipwireless) doing anything with TCFLSH
>> Since driver 'input buffer' flushing is not currently supported by the
>> core, this seems the only available workaround.
>
> That is true. But I doubt we should use these two staging drivers as a
> model for how this should be handled, if it's at all needed.
Right. My comments implied approval of the design, which I don't.
The main problem with the existing design is that it allows for a
significant variation in how ioctl(WHATEVER) is handled by various
driver/ldisc combinations. That's a bad thing because it makes
audit/review really time-consuming and makes changes prone to userspace
regressions.
Regards,
Peter Hurley
--
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
