Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 12 Jun 2017 20:49:36 +0300 | Subject | Re: [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
| |