linux-kernel - Re: [PATCH 4/6] tegra, serial8250: add ->handle_break() uart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABE8wwvfqEgCqfMKTpLVfsYqJ5KaxAKkUd67uoEfFLMes1SrFg@mail.gmail.com>
Date:	Fri, 6 Apr 2012 15:25:11 -0700
From:	"Williams, Dan J" <dan.j.williams@...el.com>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	gregkh@...uxfoundation.org,
	Sudhakar Mamillapalli <sudhakar@...com>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	Colin Cross <ccross@...roid.com>,
	Olof Johansson <olof@...om.net>,
	Nhan H Mai <nhan.h.mai@...el.com>,
	Alan Cox <alan@...rguk.ukuu.org.uk>, alan@...ux.intel.com
Subject: Re: [PATCH 4/6] tegra, serial8250: add ->handle_break() uart_port op

On Fri, Apr 6, 2012 at 2:56 PM, Stephen Warren <swarren@...dotorg.org> wrote:
> On 04/06/2012 03:28 PM, Williams, Dan J wrote:
>> On Fri, Apr 6, 2012 at 2:01 PM, Stephen Warren <swarren@...dotorg.org> wrote:
>>> On 04/06/2012 12:49 PM, Dan Williams wrote:
>>>> The "KT" serial port has another use case for a "received break" quirk,
>>>> so before adding another special case to the 8250 core take this
>>>> opportunity to push such quirks out of the core and into a uart_port op.
>>>
>>> This doesn't seem quite right. Why do the board files have to set up
>>> this .handle_break function; they're already setting .type=PORT_TEGRA,
>>> which should be enough to drive the setup of any required quirks.
>>
>> Because struct serial8250_config does not convey any uart_port ops.
>
> But couldn't it be enhanced to do so, just like this patch added a field
> to struct uart_port for this? If you went this route, then the change
> would be entirely isolated within 8250.c, so you could drop all the
> arch/arm/mach-tegra changes, and also not need to update of_serial.c.

That's kind of the point, it seems the goal of serial port hardware
design is to find new and interesting ways to make them fail.  Putting
everyone's fail in 8250.c is messy so when we tried to add another
"handle_break" quirk we were rightly asked to make the core interface
generic and push the logic out of the core.

I don't see how moving quirk routines into serial8250_config makes
this any cleaner, we'd either be left with two methods for specifying
quirks, or requiring custom serial8250_configs for drivers that only
want to set one of the ops.

Hmm, is there any precendent for an openfirmware serial port that
needs one of these custom ops?

        unsigned int            (*serial_in)(struct uart_port *, int);
        void                    (*serial_out)(struct uart_port *, int, int);
        void                    (*set_termios)(struct uart_port *,
                                               struct ktermios *new,
                                               struct ktermios *old);
        int                     (*handle_irq)(struct uart_port *);
        void                    (*pm)(struct uart_port *, unsigned int state,
                                      unsigned int old);
        void                    (*handle_break)(struct uart_port *);

...I would be surprised if this is the first time this has come up
given the prevalence of broken uarts.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/