[PATCH 3/3] serial: msm: calculate bit clock divider
Caleb Connolly
caleb.connolly at linaro.org
Mon Apr 15 16:18:07 CEST 2024
On 15/04/2024 14:05, Robert Marko wrote:
> 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.
Ok awesome, thanks for trying this out. I'll send a v2 with your suggestion.
Can I add your Tested-by?
>
> 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
>>
>
>
--
// Caleb (they/them)
More information about the U-Boot
mailing list