Re: [PATCH] tty: serial: qcom_geni_serial: Address follow-up comments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Karthik,

On Fri, Mar 23, 2018 at 3:10 PM Karthikeyan Ramasubramanian <
kramasub@xxxxxxxxxxxxxx> wrote:

> The driver has some follow-up comments right after it got merged. This
> patch addresses those comments that got missed out.

> * Document reason for newline character counting in console_write
> * Document reason for disabling IRQ in the system resume operation
> * Use min3 to find the minimum of 3 values
> * Remove unnecessary casting while using min_t
> * Use iowrite32_rep to write to the hardware FIFO
> * Initialize the console port statically
> * Fine-tune memory barrier usage

> Signed-off-by: Karthikeyan Ramasubramanian <kramasub@xxxxxxxxxxxxxx>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 84
++++++++++++++++++-----------------
>   1 file changed, 44 insertions(+), 40 deletions(-)

> diff --git a/drivers/tty/serial/qcom_geni_serial.c
b/drivers/tty/serial/qcom_geni_serial.c
> index 1442777..f5b9cb8 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
...
> @@ -406,20 +421,18 @@ static void qcom_geni_serial_start_tx(struct
uart_port *uport)
>          u32 status;

>          if (port->xfer_mode == GENI_SE_FIFO) {
> -               status = readl_relaxed(uport->membase + SE_GENI_STATUS);
> +               /*
> +                * readl ensures reading & writing of IRQ_EN register
> +                * is not re-ordered before checking the status of the
> +                * Serial Engine.
> +                */
> +               status = readl(uport->membase + SE_GENI_STATUS);
>                  if (status & M_GENI_CMD_ACTIVE)
>                          return;

>                  if (!qcom_geni_serial_tx_empty(uport))
>                          return;

> -               /*
> -                * Ensure writing to IRQ_EN & watermark registers are not
> -                * re-ordered before checking the status of the Serial
> -                * Engine and TX FIFO
> -                */
> -               mb();
> -
>                  irq_en = readl_relaxed(uport->membase + SE_GENI_M_IRQ_EN);
>                  irq_en |= M_TX_FIFO_WATERMARK_EN | M_CMD_DONE_EN;


Thanks for doing this fixup in start_tx. The same pattern would apply in
stop_tx, start_rx, and stop_rx, right?

-Evan
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux