On Mon, Jun 12, 2017 at 6:37 PM, Dong Aisheng [off-list ref] wrote:

This is used to dynamically check the SoC specific lpuart properies.
Currently only the iotype is added, it functions the same as before.
With this, new chips with different iotype will be more easily added.

+struct lpuart_soc_data {
+       char    iotype;
+};
+
+static const struct lpuart_soc_data vf_data = {
+       .iotype = UPIO_MEM,
+};
+
+static const struct lpuart_soc_data ls_data = {
+       .iotype = UPIO_MEM32BE,

+

Redundant.

+};

And now most interesting part...

-               if (sport->lpuart32)
+               if (sport->port.iotype & UPIO_MEM32BE)
                        lpuart32_write(sport->port.x_char, sport->port.membase + UARTDATA);
                else
                        writeb(sport->port.x_char, sport->port.membase + UARTDR);

-               if (sport->lpuart32)
+               if (sport->port.iotype & UPIO_MEM32BE)
                        lpuart32_stop_tx(&sport->port);
                else
                        lpuart_stop_tx(&sport->port);

-       if (sport->lpuart32)
+       if (sport->port.iotype & UPIO_MEM32BE)
                lpuart32_transmit_buffer(sport);
        else
                lpuart_transmit_buffer(sport);

-               if (sport->lpuart32)
+               if (sport->port.iotype & UPIO_MEM32BE)
                        lpuart32_console_get_options(sport, &baud, &parity, &bits);
                else
                        lpuart_console_get_options(sport, &baud, &parity, &bits);

-       if (sport->lpuart32)
+       if (sport->port.iotype & UPIO_MEM32BE)
                lpuart32_setup_watermark(sport);
        else
                lpuart_setup_watermark(sport);

-       if (sport->lpuart32)
+       sport->port.iotype = sdata->iotype;
+       if (sport->port.iotype & UPIO_MEM32BE)
                sport->port.ops = &lpuart32_pops;
        else
                sport->port.ops = &lpuart_pops;

-       if (sport->lpuart32)
+       if (sport->port.iotype & UPIO_MEM32BE)
                lpuart_reg.cons = LPUART32_CONSOLE;
        else
                lpuart_reg.cons = LPUART_CONSOLE;

...all above since you introduced nice struct, can get rid of conditionals.
Instead it might be a members of the struct above.

(I dunno if it's good to have in this patch, but at list a follow up
could be nice to have)

-       if (sport->lpuart32) {
+       if (sport->port.iotype & UPIO_MEM32BE) {
                /* disable Rx/Tx and interrupts */
                temp = lpuart32_read(sport->port.membase + UARTCTRL);
                temp &= ~(UARTCTRL_TE | UARTCTRL_TIE | UARTCTRL_TCIE);

-       if (sport->lpuart32) {
+       if (sport->port.iotype & UPIO_MEM32BE) {
                lpuart32_setup_watermark(sport);
                temp = lpuart32_read(sport->port.membase + UARTCTRL);
                temp |= (UARTCTRL_RIE | UARTCTRL_TIE | UARTCTRL_RE |

Above are questionable, might be not need to convert them.

So, in any case above is a sighting which you could address (separately).

-- 
With Best Regards,
Andy Shevchenko