[PATCH 3/3] serial: msm: calculate bit clock divider

Robert Marko robert.marko at sartura.hr
Mon Apr 15 15:05:23 CEST 2024


On Mon, Apr 15, 2024 at 2:44 PM Caleb Connolly
<caleb.connolly at linaro.org> wrote:
>
> The driver currently requires the bit clock divider be hardcoded in
> devicetree (or use the hardcoded default from apq8016).
>
> The bit clock divider is used to derive the baud rate from the core
> clock:
>
>   baudrate = clk_rate / csr_div
>
> clk_rate is the actual programmed core clock rate which is returned by
> clk_set_rate(), and this UART driver only supports a baudrate of 115200.
> We can therefore determine the appropriate value for UARTDM_CSR by
> iterating over the possible values and finding the one where the
> equation above holds true for a baudrate of 115200.
>
> Implement this logic and drop the non-standard DT bindings for this
> driver.
>
> Tested on dragonboard410c.
>
> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>

Works on Alfa AP120C (IPQ4018) with full DM UART, but debug UART
prints junk since .clk_rate = 7372800 is not correct for IPQ40xx.
I would suggest using .clk_rate = CONFIG_VAL(DEBUG_UART_CLOCK) instead
to populate the value per board, this also avoids per ARCH ifdefs.

Regards,
Robert
> ---
> Cc: Robert Marko <robert.marko at sartura.hr>
> ---
>  doc/device-tree-bindings/serial/msm-serial.txt | 10 ---
>  drivers/serial/serial_msm.c                    | 87 +++++++++++++++++++++-----
>  2 files changed, 70 insertions(+), 27 deletions(-)
>
> diff --git a/doc/device-tree-bindings/serial/msm-serial.txt b/doc/device-tree-bindings/serial/msm-serial.txt
> deleted file mode 100644
> index dca995798a90..000000000000
> --- a/doc/device-tree-bindings/serial/msm-serial.txt
> +++ /dev/null
> @@ -1,10 +0,0 @@
> -Qualcomm UART (Data Mover mode)
> -
> -Required properties:
> -- compatible: must be "qcom,msm-uartdm-v1.4"
> -- reg: start address and size of the registers
> -- clock: interface clock (must accept baudrate as a frequency)
> -
> -Optional properties:
> -- bit-rate: Data Mover bit rate register value
> -                       (If not defined then 0xCC is used as default)
> diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
> index 8044d38518db..e461929b4338 100644
> --- a/drivers/serial/serial_msm.c
> +++ b/drivers/serial/serial_msm.c
> @@ -31,8 +31,18 @@
>  #define UARTDM_RXFS_BUF_SHIFT   0x7  /* Number of bytes in the packing buffer */
>  #define UARTDM_RXFS_BUF_MASK    0x7
>  #define UARTDM_MR1                              0x00
>  #define UARTDM_MR2                              0x04
> +/*
> + * This is documented on page 1817 of the apq8016e technical reference manual.
> + * section 6.2.5.3.26
> + *
> + * The upper nybble contains the bit clock divider for the RX pin, the lower
> + * nybble defines the TX pin. In almost all cases these should be the same value.
> + *
> + * The baud rate is the core clock frequency divided by the fixed divider value
> + * programmed into this register (defined in calc_csr_bitrate()).
> + */
>  #define UARTDM_CSR                              0xA0
>
>  #define UARTDM_SR                0xA4 /* Status register */
>  #define UARTDM_SR_RX_READY       (1 << 0) /* Word is the receiver FIFO */
> @@ -52,9 +62,8 @@
>
>  #define UARTDM_TF               0x100 /* UART Transmit FIFO register */
>  #define UARTDM_RF               0x140 /* UART Receive FIFO register */
>
> -#define UART_DM_CLK_RX_TX_BIT_RATE 0xCC
>  #define MSM_BOOT_UART_DM_8_N_1_MODE 0x34
>  #define MSM_BOOT_UART_DM_CMD_RESET_RX 0x10
>  #define MSM_BOOT_UART_DM_CMD_RESET_TX 0x20
>
> @@ -63,9 +72,9 @@ DECLARE_GLOBAL_DATA_PTR;
>  struct msm_serial_data {
>         phys_addr_t base;
>         unsigned chars_cnt; /* number of buffered chars */
>         uint32_t chars_buf; /* buffered chars */
> -       uint32_t clk_bit_rate; /* data mover mode bit rate register value */
> +       uint32_t clk_rate; /* core clock rate */
>  };
>
>  static int msm_serial_fetch(struct udevice *dev)
>  {
> @@ -155,34 +164,63 @@ static const struct dm_serial_ops msm_serial_ops = {
>         .pending = msm_serial_pending,
>         .getc = msm_serial_getc,
>  };
>
> -static int msm_uart_clk_init(struct udevice *dev)
> +static long msm_uart_clk_init(struct udevice *dev)
>  {
> -       uint clk_rate = fdtdec_get_uint(gd->fdt_blob, dev_of_offset(dev),
> -                                       "clock-frequency", 115200);
> +       struct msm_serial_data *priv = dev_get_priv(dev);
>         struct clk clk;
>         int ret;
> +       long rate;
>
>         ret = clk_get_by_name(dev, "core", &clk);
>         if (ret < 0) {
>                 pr_warn("%s: Failed to get clock: %d\n", __func__, ret);
> -               return ret;
> +               return 0;
>         }
>
> -       ret = clk_set_rate(&clk, clk_rate);
> -       if (ret < 0)
> -               return ret;
> +       rate = clk_set_rate(&clk, priv->clk_rate);
>
> -       return 0;
> +       return rate;
> +}
> +
> +static int calc_csr_bitrate(struct msm_serial_data *priv)
> +{
> +       /* This table is from the TRE. See the definition of UARTDM_CSR */
> +       unsigned int csr_div_table[] = {24576, 12288, 6144, 3072, 1536, 768, 512, 384,
> +                                       256,   192,   128,  96,   64,   48,  32,  16};
> +       int i = ARRAY_SIZE(csr_div_table) - 1;
> +       /* Currently we only support one baudrate */
> +       int baud = 115200;
> +
> +       for (; i >= 0; i--) {
> +               int x = priv->clk_rate / csr_div_table[i];
> +
> +               if (x == baud)
> +                       /* Duplicate the configuration for RX
> +                        * as the lower nybble only configures TX
> +                        */
> +                       return i + (i << 4);
> +       }
> +
> +       return -EINVAL;
>  }
>
>  static void uart_dm_init(struct msm_serial_data *priv)
>  {
>         /* Delay initialization for a bit to let pins stabilize if necessary */
>         mdelay(5);
> +       int bitrate = calc_csr_bitrate(priv);
> +       if (bitrate < 0) {
> +               log_warning("Couldn't calculate bit clock divider! Using default\n");
> +               /* This happens to be the value used on MSM8916 for the hardcoded clockrate
> +                * in clock-apq8016. It's at least a better guess than a value we *know*
> +                * is wrong...
> +                */
> +               bitrate = 0xCC;
> +       }
>
> -       writel(priv->clk_bit_rate, priv->base + UARTDM_CSR);
> +       writel(bitrate, priv->base + UARTDM_CSR);
>         writel(0x0, priv->base + UARTDM_MR1);
>         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
>         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
>         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> @@ -191,18 +229,27 @@ static void uart_dm_init(struct msm_serial_data *priv)
>         writel(0x0, priv->base + UARTDM_DMEN);
>  }
>  static int msm_serial_probe(struct udevice *dev)
>  {
> -       int ret;
>         struct msm_serial_data *priv = dev_get_priv(dev);
> +       long rate;
>
>         /* No need to reinitialize the UART after relocation */
>         if (gd->flags & GD_FLG_RELOC)
>                 return 0;
>
> -       ret = msm_uart_clk_init(dev);
> -       if (ret)
> -               return ret;
> +       rate = msm_uart_clk_init(dev);
> +       if (rate < 0)
> +               return rate;
> +       if (!rate) {
> +               log_err("Got core clock rate of 0... Please fix your clock driver\n");
> +               return -EINVAL;
> +       }
> +
> +       /* Update the clock rate to the actual programmed rate returned by the
> +        * clock driver
> +        */
> +       priv->clk_rate = rate;
>
>         uart_dm_init(priv);
>
>         return 0;
> @@ -210,15 +257,20 @@ static int msm_serial_probe(struct udevice *dev)
>
>  static int msm_serial_of_to_plat(struct udevice *dev)
>  {
>         struct msm_serial_data *priv = dev_get_priv(dev);
> +       int ret;
>
>         priv->base = dev_read_addr(dev);
>         if (priv->base == FDT_ADDR_T_NONE)
>                 return -EINVAL;
>
> -       priv->clk_bit_rate = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev),
> -                                                       "bit-rate", UART_DM_CLK_RX_TX_BIT_RATE);
> +       ret = dev_read_u32(dev, "clock-frequency", &priv->clk_rate);
> +       if (ret < 0) {
> +               log_debug("No clock frequency specified, using default rate\n");
> +               /* Default for APQ8016 */
> +               priv->clk_rate = 7372800;
> +       }
>
>         return 0;
>  }
>
> @@ -241,8 +293,9 @@ U_BOOT_DRIVER(serial_msm) = {
>  #ifdef CONFIG_DEBUG_UART_MSM
>
>  static struct msm_serial_data init_serial_data = {
>         .base = CONFIG_VAL(DEBUG_UART_BASE),
> +       .clk_rate = 7372800,
>  };
>
>  #include <debug_uart.h>
>
>
> --
> 2.44.0
>


-- 
Robert Marko
Staff Embedded Linux Engineer
Sartura Ltd.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko at sartura.hr
Web: www.sartura.hr


More information about the U-Boot mailing list