lkml.org 
[lkml]   [2017]   [Jun]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH V3 1/7] tty: serial: lpuart: introduce lpuart_soc_data to represent SoC property
On Mon, Jun 12, 2017 at 6:37 PM, Dong Aisheng <aisheng.dong@nxp.com> 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

\
 
 \ /
  Last update: 2017-06-12 20:57    [W:0.105 / U:0.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site