On 03/06/2015 12:29 PM, Mason wrote:
> Måns Rullgård wrote:
>
>> Mason wrote:
>>
>>> I am working on a Tango4 SoC, which has the same "weird register layout"
>>> as Au1x00/RT288x UART. The verilog says "The uartcore is compatible with
>>> National 16550D UART", and the doc says "The UART is almost compatible
>>> with National Semiconductor's 16550".
>>>
>>> I have a few questions with respect to 8250_core.c
>>>
>>> In no particular order:
>>>
>>> (I'm having a hard time following the function pointers.)
>>> I don't think serial8250_config_port() calls autoconfig() right?
>>> i.e. I don't think UART_CONFIG_TYPE is set when port->iotype == UPIO_AU
>>>
>>> I don't see where port.type is set, and to what (PORT_16550A?)
>>> And how fifo_size and tx_loadsz are set up?
>>
>> Some parameters can be configured in your device tree entry for the UART,
>> see of_serial.c.
>
> I was wondering about the default case, i.e. using the values provided
> by the uart_config array, for example .fifo_size = .tx_loadsz = 16 in
> the PORT_16550A case.
>
>>> The PORT_16550A entry doesn't set UART_CAP_EFR, yet autoconfig_16550a()
>>> tries to read UART_EFR... how come?
>>
>> To check for a couple of variants that do have such a register, just
>> like the comments say.
>
> Is the UART_CONFIG_TYPE flag set for an UPIO_AU iotype?
>
> The only place that sets UART_CONFIG_TYPE is
> drivers/tty/serial/serial_core.c:2177
>
> if (port->flags & UPF_BOOT_AUTOCONF) {
> if (!(port->flags & UPF_FIXED_TYPE)) {
> port->type = PORT_UNKNOWN;
> flags |= UART_CONFIG_TYPE;
> }
> port->ops->config_port(port, flags);
> }
>
> Don't know where to look at that point.
For exactly this scenario, the 8250 core rechecks the fields
derived from port->type (fifosize, capabilites, etc) when the port
is first opened in serial8250_do_startup().
>>> The RT3050 (which is supposed to be compatible with the RT288x?) has
>>> a scratch register. But the au_io_{in,out}_map lookup tables don't
>>> map UART_SCR, and some functions seem to use the scratch register,
>>> such as serial8250_console_write
>>
>> The scratch register is only used for two things:
>
> Whatever it is used for, here's the problem:
>
> serial_port_in(port, UART_SCR); calls au_serial_in(port, UART_SCR);
>
> Out-of-bound access.
That's my breakage.
> serial_out(up, UART_SCR, canary); is even worse, as it will scribble
> over some innocent bystander.
It won't be a different device because the mmio resource request is
for base + 8 registers.
>> 1. To check if the scratch register exists in order to distinguish an
>> original 8250 from a 16450.
>>
>> 2. To tell if the UART lost state during a suspend/resume cycle. If the
>> scratch register doesn't exist, the port will simply be reinitialised
>> same as if it existed and got corrupted while suspended.
>>
>> The reason UART_SCR isn't mapped in the tables is probably that the
>> Alchemy, for which this was originally written, doesn't have a scratch
>> register (offset 0x24 has a different function entirely there), and
>> nothing important depends on it for the Ralink ones. The driver works
>> as is for Tango3, so odds are it will work for Tango4 as well.
>
> So an appropriate fix might be:
>
> --- 8250_core.c 2015-03-03 18:04:59.000000000 +0100
> +++ 8250_core.fix.c 2015-03-06 18:23:27.374740156 +0100
> @@ -378,12 +378,14 @@
>
> static unsigned int au_serial_in(struct uart_port *p, int offset)
> {
> + if (offset > UART_MSR) return 0;
> offset = au_io_in_map[offset] << p->regshift;
> return __raw_readl(p->membase + offset);
> }
>
> static void au_serial_out(struct uart_port *p, int offset, int value)
> {
> + if (offset > UART_MCR) return;
> offset = au_io_out_map[offset] << p->regshift;
> __raw_writel(value, p->membase + offset);
> }
If that fixes my breakage, then ok. I could also skip this UPIO_* type
for the canary/console restore problem.
Will you submit that as a patch or would you like me to?
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
