diff mbox series

[v2,2/5] apq8016: Add support for UART1 clocks and pinmux

Message ID 20240311111027.44577-3-sumit.garg@linaro.org
State Superseded
Delegated to: Caleb Connolly
Headers show
Series Add SE HMBSC board support | expand

Commit Message

Sumit Garg March 11, 2024, 11:10 a.m. UTC
SE HMIBSC board uses UART1 as the main debug console, so add
corresponding clocks and pinmux support. Along with that update
instructions to enable clocks for debug UART support.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 drivers/clk/qcom/clock-apq8016.c       | 50 +++++++++++++++++++++-----
 drivers/pinctrl/qcom/pinctrl-apq8016.c |  1 +
 drivers/serial/serial_msm.c            |  6 ++--
 3 files changed, 47 insertions(+), 10 deletions(-)

Comments

Caleb Connolly March 11, 2024, 12:27 p.m. UTC | #1
Hi Sumit,

On 11/03/2024 11:10, Sumit Garg wrote:
> SE HMIBSC board uses UART1 as the main debug console, so add
> corresponding clocks and pinmux support. Along with that update
> instructions to enable clocks for debug UART support.
> 
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>  drivers/clk/qcom/clock-apq8016.c       | 50 +++++++++++++++++++++-----
>  drivers/pinctrl/qcom/pinctrl-apq8016.c |  1 +
>  drivers/serial/serial_msm.c            |  6 ++--
>  3 files changed, 47 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
> index e6647f7c41d..a620a10a520 100644
> --- a/drivers/clk/qcom/clock-apq8016.c
> +++ b/drivers/clk/qcom/clock-apq8016.c
> @@ -43,6 +43,14 @@
>  #define BLSP1_UART2_APPS_N		(0x3040)
>  #define BLSP1_UART2_APPS_D		(0x3044)
>  
> +#define BLSP1_UART1_BCR			(0x2038)
> +#define BLSP1_UART1_APPS_CBCR		(0x203C)
> +#define BLSP1_UART1_APPS_CMD_RCGR	(0x2044)
> +#define BLSP1_UART1_APPS_CFG_RCGR	(0x2048)
> +#define BLSP1_UART1_APPS_M		(0x204C)
> +#define BLSP1_UART1_APPS_N		(0x2050)
> +#define BLSP1_UART1_APPS_D		(0x2054)
> +
>  /* GPLL0 clock control registers */
>  #define GPLL0_STATUS_ACTIVE BIT(17)
>  
> @@ -77,7 +85,7 @@ static struct vote_clk gcc_blsp1_ahb_clk = {
>  };
>  
>  /* SDHCI */
> -static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
> +static int apq8016_clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
This seems like an unrelated change, I don't think we need to namespace
this function as it's static.
>  {
>  	int div = 15; /* 100MHz default */
>  
> @@ -94,6 +102,33 @@ static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
>  	return rate;
>  }
>  
> +static const struct bcr_regs uart1_regs = {
> +	.cfg_rcgr = BLSP1_UART1_APPS_CFG_RCGR,
> +	.cmd_rcgr = BLSP1_UART1_APPS_CMD_RCGR,
> +	.M = BLSP1_UART1_APPS_M,
> +	.N = BLSP1_UART1_APPS_N,
> +	.D = BLSP1_UART1_APPS_D,
> +};
> +
> +/* UART: 115200 */
> +static int apq8016_clk_init_uart1(phys_addr_t base)

I know we're still dealing with some tech debt here, but I'm not a big
fan of this approach. I notice that the RCG and CBCR registers are all
offset by exactly 0xff0 between UART1 and UART2, what about adding a
second "index" parameter to apq8016_clk_init_uart() and then offsetting
the addresses by (0xff0 * index)?

This will get cleaner once we drop the bcr_regs struct.
> +{
> +	/* Enable AHB clock */
> +	clk_enable_vote_clk(base, &gcc_blsp1_ahb_clk);
> +
> +	/* 7372800 uart block clock @ GPLL0 */
> +	clk_rcg_set_rate_mnd(base, &uart1_regs, 1, 144, 15625,
> +			     CFG_CLK_SRC_GPLL0, 16);
> +
> +	/* Vote for gpll0 clock */
> +	clk_enable_gpll0(base, &gpll0_vote_clk);
> +
> +	/* Enable core clk */
> +	clk_enable_cbc(base + BLSP1_UART1_APPS_CBCR);
> +
> +	return 0;
> +}
> +
>  static const struct bcr_regs uart2_regs = {
>  	.cfg_rcgr = BLSP1_UART2_APPS_CFG_RCGR,
>  	.cmd_rcgr = BLSP1_UART2_APPS_CMD_RCGR,
> @@ -103,7 +138,7 @@ static const struct bcr_regs uart2_regs = {
>  };
>  
>  /* UART: 115200 */
> -int apq8016_clk_init_uart(phys_addr_t base)
> +int apq8016_clk_init_uart2(phys_addr_t base)
>  {
>  	/* Enable AHB clock */
>  	clk_enable_vote_clk(base, &gcc_blsp1_ahb_clk);
> @@ -127,14 +162,13 @@ static ulong apq8016_clk_set_rate(struct clk *clk, ulong rate)
>  
>  	switch (clk->id) {
>  	case GCC_SDCC1_APPS_CLK: /* SDC1 */
> -		return clk_init_sdc(priv, 0, rate);
> -		break;
> +		return apq8016_clk_init_sdc(priv, 0, rate);
>  	case GCC_SDCC2_APPS_CLK: /* SDC2 */
> -		return clk_init_sdc(priv, 1, rate);
> -		break;
> +		return apq8016_clk_init_sdc(priv, 1, rate);
>  	case GCC_BLSP1_UART2_APPS_CLK: /* UART2 */
> -		return apq8016_clk_init_uart(priv->base);
> -		break;
> +		return apq8016_clk_init_uart2(priv->base);
> +	case GCC_BLSP1_UART1_APPS_CLK: /* UART1 */
> +		return apq8016_clk_init_uart1(priv->base);
>  	default:
>  		return 0;
>  	}
> diff --git a/drivers/pinctrl/qcom/pinctrl-apq8016.c b/drivers/pinctrl/qcom/pinctrl-apq8016.c
> index db0e2124684..cb0e2227079 100644
> --- a/drivers/pinctrl/qcom/pinctrl-apq8016.c
> +++ b/drivers/pinctrl/qcom/pinctrl-apq8016.c
> @@ -29,6 +29,7 @@ static const char * const msm_pinctrl_pins[] = {
>  };
>  
>  static const struct pinctrl_function msm_pinctrl_functions[] = {
> +	{"blsp_uart1", 2},
>  	{"blsp_uart2", 2},
>  };
>  
> diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
> index ac4280c6c4c..eaf024a55b0 100644
> --- a/drivers/serial/serial_msm.c
> +++ b/drivers/serial/serial_msm.c
> @@ -248,12 +248,14 @@ static struct msm_serial_data init_serial_data = {
>  #include <debug_uart.h>
>  
>  /* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
Please update the comment to offer some hints about which UART should be
enabled.
> -//int apq8016_clk_init_uart(phys_addr_t gcc_base);
> +//int apq8016_clk_init_uart1(phys_addr_t gcc_base);
> +//int apq8016_clk_init_uart2(phys_addr_t gcc_base);
>  
>  static inline void _debug_uart_init(void)
>  {
>  	/* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
> -	//apq8016_clk_init_uart(0x1800000);
> +	//apq8016_clk_init_uart1(0x1800000);
> +	//apq8016_clk_init_uart2(0x1800000);
>  	uart_dm_init(&init_serial_data);
>  }
>
Stephan Gerhold March 11, 2024, 1:32 p.m. UTC | #2
On Mon, Mar 11, 2024 at 12:27:11PM +0000, Caleb Connolly wrote:
> On 11/03/2024 11:10, Sumit Garg wrote:
> > SE HMIBSC board uses UART1 as the main debug console, so add
> > corresponding clocks and pinmux support. Along with that update
> > instructions to enable clocks for debug UART support.
> > 
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >  drivers/clk/qcom/clock-apq8016.c       | 50 +++++++++++++++++++++-----
> >  drivers/pinctrl/qcom/pinctrl-apq8016.c |  1 +
> >  drivers/serial/serial_msm.c            |  6 ++--
> >  3 files changed, 47 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
> > index e6647f7c41d..a620a10a520 100644
> > --- a/drivers/clk/qcom/clock-apq8016.c
> > +++ b/drivers/clk/qcom/clock-apq8016.c
> > @@ -43,6 +43,14 @@
> >  #define BLSP1_UART2_APPS_N		(0x3040)
> >  #define BLSP1_UART2_APPS_D		(0x3044)
> >  
> > +#define BLSP1_UART1_BCR			(0x2038)
> > +#define BLSP1_UART1_APPS_CBCR		(0x203C)
> > +#define BLSP1_UART1_APPS_CMD_RCGR	(0x2044)
> > +#define BLSP1_UART1_APPS_CFG_RCGR	(0x2048)
> > +#define BLSP1_UART1_APPS_M		(0x204C)
> > +#define BLSP1_UART1_APPS_N		(0x2050)
> > +#define BLSP1_UART1_APPS_D		(0x2054)
> > +
> >  /* GPLL0 clock control registers */
> >  #define GPLL0_STATUS_ACTIVE BIT(17)
> >  
> > [...]
> > @@ -94,6 +102,33 @@ static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
> >  	return rate;
> >  }
> >  
> > +static const struct bcr_regs uart1_regs = {
> > +	.cfg_rcgr = BLSP1_UART1_APPS_CFG_RCGR,
> > +	.cmd_rcgr = BLSP1_UART1_APPS_CMD_RCGR,
> > +	.M = BLSP1_UART1_APPS_M,
> > +	.N = BLSP1_UART1_APPS_N,
> > +	.D = BLSP1_UART1_APPS_D,
> > +};
> > +
> > +/* UART: 115200 */
> > +static int apq8016_clk_init_uart1(phys_addr_t base)
> 
> I know we're still dealing with some tech debt here, but I'm not a big
> fan of this approach. I notice that the RCG and CBCR registers are all
> offset by exactly 0xff0 between UART1 and UART2, what about adding a
> second "index" parameter to apq8016_clk_init_uart() and then offsetting
> the addresses by (0xff0 * index)?
> 

This would work for MSM8916 where you have just two UARTs, but it might
be misleading since the UART blocks are actually separated in 4 KiB
(0x1000) blocks and not offset by 0xff0. UART2 is a special case that
has different offsets within the 4 KiB block, for some weird reason.

If you want to calculate the register offsets properly you would need
special handling for UART2, e.g. I have the following (admittedly rather
ugly) define in the TF-A port for MSM8916 and similar [1]:

#define GCC_BLSP1_UART_APPS_CBCR(n)	(GCC_BASE + \
	(((n) == 2) ? (0x0302c) : (0x0203c + (((n) - 1) * 0x1000))))

where n is the UART number (here 1 or 2). As a different example,
MDM9607 has 5 UARTs (1 to 5) and there only UART2 is special-cased,
while all others follow the standard offset with 0x1000 offset.

Thanks,
Stephan

[1]: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/plat/qti/msm8916/msm8916_setup.c?h=v2.10#n40
Caleb Connolly March 11, 2024, 2:50 p.m. UTC | #3
On 11/03/2024 13:32, Stephan Gerhold wrote:
> On Mon, Mar 11, 2024 at 12:27:11PM +0000, Caleb Connolly wrote:
>> On 11/03/2024 11:10, Sumit Garg wrote:
>>> SE HMIBSC board uses UART1 as the main debug console, so add
>>> corresponding clocks and pinmux support. Along with that update
>>> instructions to enable clocks for debug UART support.
>>>
>>> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
>>> ---
>>>  drivers/clk/qcom/clock-apq8016.c       | 50 +++++++++++++++++++++-----
>>>  drivers/pinctrl/qcom/pinctrl-apq8016.c |  1 +
>>>  drivers/serial/serial_msm.c            |  6 ++--
>>>  3 files changed, 47 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
>>> index e6647f7c41d..a620a10a520 100644
>>> --- a/drivers/clk/qcom/clock-apq8016.c
>>> +++ b/drivers/clk/qcom/clock-apq8016.c
>>> @@ -43,6 +43,14 @@
>>>  #define BLSP1_UART2_APPS_N		(0x3040)
>>>  #define BLSP1_UART2_APPS_D		(0x3044)
>>>  
>>> +#define BLSP1_UART1_BCR			(0x2038)
>>> +#define BLSP1_UART1_APPS_CBCR		(0x203C)
>>> +#define BLSP1_UART1_APPS_CMD_RCGR	(0x2044)
>>> +#define BLSP1_UART1_APPS_CFG_RCGR	(0x2048)
>>> +#define BLSP1_UART1_APPS_M		(0x204C)
>>> +#define BLSP1_UART1_APPS_N		(0x2050)
>>> +#define BLSP1_UART1_APPS_D		(0x2054)
>>> +
>>>  /* GPLL0 clock control registers */
>>>  #define GPLL0_STATUS_ACTIVE BIT(17)
>>>  
>>> [...]
>>> @@ -94,6 +102,33 @@ static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
>>>  	return rate;
>>>  }
>>>  
>>> +static const struct bcr_regs uart1_regs = {
>>> +	.cfg_rcgr = BLSP1_UART1_APPS_CFG_RCGR,
>>> +	.cmd_rcgr = BLSP1_UART1_APPS_CMD_RCGR,
>>> +	.M = BLSP1_UART1_APPS_M,
>>> +	.N = BLSP1_UART1_APPS_N,
>>> +	.D = BLSP1_UART1_APPS_D,
>>> +};
>>> +
>>> +/* UART: 115200 */
>>> +static int apq8016_clk_init_uart1(phys_addr_t base)
>>
>> I know we're still dealing with some tech debt here, but I'm not a big
>> fan of this approach. I notice that the RCG and CBCR registers are all
>> offset by exactly 0xff0 between UART1 and UART2, what about adding a
>> second "index" parameter to apq8016_clk_init_uart() and then offsetting
>> the addresses by (0xff0 * index)?
>>
> 
> This would work for MSM8916 where you have just two UARTs, but it might
> be misleading since the UART blocks are actually separated in 4 KiB
> (0x1000) blocks and not offset by 0xff0. UART2 is a special case that
> has different offsets within the 4 KiB block, for some weird reason.
> 
> If you want to calculate the register offsets properly you would need
> special handling for UART2, e.g. I have the following (admittedly rather
> ugly) define in the TF-A port for MSM8916 and similar [1]:
> 
> #define GCC_BLSP1_UART_APPS_CBCR(n)	(GCC_BASE + \
> 	(((n) == 2) ? (0x0302c) : (0x0203c + (((n) - 1) * 0x1000))))
> 
> where n is the UART number (here 1 or 2). As a different example,
> MDM9607 has 5 UARTs (1 to 5) and there only UART2 is special-cased,
> while all others follow the standard offset with 0x1000 offset.

fwiw I would also be fine with just treating it like a binary switch and
only allowing UART1 or UART2 to be selected if that's simpler.
> 
> Thanks,
> Stephan
> 
> [1]: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/plat/qti/msm8916/msm8916_setup.c?h=v2.10#n40
Sumit Garg March 12, 2024, 8:15 a.m. UTC | #4
On Mon, 11 Mar 2024 at 20:20, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
>
>
> On 11/03/2024 13:32, Stephan Gerhold wrote:
> > On Mon, Mar 11, 2024 at 12:27:11PM +0000, Caleb Connolly wrote:
> >> On 11/03/2024 11:10, Sumit Garg wrote:
> >>> SE HMIBSC board uses UART1 as the main debug console, so add
> >>> corresponding clocks and pinmux support. Along with that update
> >>> instructions to enable clocks for debug UART support.
> >>>
> >>> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> >>> ---
> >>>  drivers/clk/qcom/clock-apq8016.c       | 50 +++++++++++++++++++++-----
> >>>  drivers/pinctrl/qcom/pinctrl-apq8016.c |  1 +
> >>>  drivers/serial/serial_msm.c            |  6 ++--
> >>>  3 files changed, 47 insertions(+), 10 deletions(-)
> >>>
> >>> diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
> >>> index e6647f7c41d..a620a10a520 100644
> >>> --- a/drivers/clk/qcom/clock-apq8016.c
> >>> +++ b/drivers/clk/qcom/clock-apq8016.c
> >>> @@ -43,6 +43,14 @@
> >>>  #define BLSP1_UART2_APPS_N         (0x3040)
> >>>  #define BLSP1_UART2_APPS_D         (0x3044)
> >>>
> >>> +#define BLSP1_UART1_BCR                    (0x2038)
> >>> +#define BLSP1_UART1_APPS_CBCR              (0x203C)
> >>> +#define BLSP1_UART1_APPS_CMD_RCGR  (0x2044)
> >>> +#define BLSP1_UART1_APPS_CFG_RCGR  (0x2048)
> >>> +#define BLSP1_UART1_APPS_M         (0x204C)
> >>> +#define BLSP1_UART1_APPS_N         (0x2050)
> >>> +#define BLSP1_UART1_APPS_D         (0x2054)
> >>> +
> >>>  /* GPLL0 clock control registers */
> >>>  #define GPLL0_STATUS_ACTIVE BIT(17)
> >>>
> >>> [...]
> >>> @@ -94,6 +102,33 @@ static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
> >>>     return rate;
> >>>  }
> >>>
> >>> +static const struct bcr_regs uart1_regs = {
> >>> +   .cfg_rcgr = BLSP1_UART1_APPS_CFG_RCGR,
> >>> +   .cmd_rcgr = BLSP1_UART1_APPS_CMD_RCGR,
> >>> +   .M = BLSP1_UART1_APPS_M,
> >>> +   .N = BLSP1_UART1_APPS_N,
> >>> +   .D = BLSP1_UART1_APPS_D,
> >>> +};
> >>> +
> >>> +/* UART: 115200 */
> >>> +static int apq8016_clk_init_uart1(phys_addr_t base)
> >>
> >> I know we're still dealing with some tech debt here, but I'm not a big
> >> fan of this approach. I notice that the RCG and CBCR registers are all
> >> offset by exactly 0xff0 between UART1 and UART2, what about adding a
> >> second "index" parameter to apq8016_clk_init_uart() and then offsetting
> >> the addresses by (0xff0 * index)?
> >>
> >
> > This would work for MSM8916 where you have just two UARTs, but it might
> > be misleading since the UART blocks are actually separated in 4 KiB
> > (0x1000) blocks and not offset by 0xff0. UART2 is a special case that
> > has different offsets within the 4 KiB block, for some weird reason.
> >
> > If you want to calculate the register offsets properly you would need
> > special handling for UART2, e.g. I have the following (admittedly rather
> > ugly) define in the TF-A port for MSM8916 and similar [1]:
> >
> > #define GCC_BLSP1_UART_APPS_CBCR(n)   (GCC_BASE + \
> >       (((n) == 2) ? (0x0302c) : (0x0203c + (((n) - 1) * 0x1000))))
> >
> > where n is the UART number (here 1 or 2). As a different example,
> > MDM9607 has 5 UARTs (1 to 5) and there only UART2 is special-cased,
> > while all others follow the standard offset with 0x1000 offset.
>
> fwiw I would also be fine with just treating it like a binary switch and
> only allowing UART1 or UART2 to be selected if that's simpler.

Let me rather pass GCC_BLSP1_UART1_APPS_CLK/GCC_BLSP1_UART2_APPS_CLK
as an argument to the apq8016_clk_init_uart().

-Sumit

> >
> > Thanks,
> > Stephan
> >
> > [1]: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/plat/qti/msm8916/msm8916_setup.c?h=v2.10#n40
>
> --
> // Caleb (they/them)
Sumit Garg March 12, 2024, 8:20 a.m. UTC | #5
On Mon, 11 Mar 2024 at 17:57, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Hi Sumit,
>
> On 11/03/2024 11:10, Sumit Garg wrote:
> > SE HMIBSC board uses UART1 as the main debug console, so add
> > corresponding clocks and pinmux support. Along with that update
> > instructions to enable clocks for debug UART support.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >  drivers/clk/qcom/clock-apq8016.c       | 50 +++++++++++++++++++++-----
> >  drivers/pinctrl/qcom/pinctrl-apq8016.c |  1 +
> >  drivers/serial/serial_msm.c            |  6 ++--
> >  3 files changed, 47 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
> > index e6647f7c41d..a620a10a520 100644
> > --- a/drivers/clk/qcom/clock-apq8016.c
> > +++ b/drivers/clk/qcom/clock-apq8016.c
> > @@ -43,6 +43,14 @@
> >  #define BLSP1_UART2_APPS_N           (0x3040)
> >  #define BLSP1_UART2_APPS_D           (0x3044)
> >
> > +#define BLSP1_UART1_BCR                      (0x2038)
> > +#define BLSP1_UART1_APPS_CBCR                (0x203C)
> > +#define BLSP1_UART1_APPS_CMD_RCGR    (0x2044)
> > +#define BLSP1_UART1_APPS_CFG_RCGR    (0x2048)
> > +#define BLSP1_UART1_APPS_M           (0x204C)
> > +#define BLSP1_UART1_APPS_N           (0x2050)
> > +#define BLSP1_UART1_APPS_D           (0x2054)
> > +
> >  /* GPLL0 clock control registers */
> >  #define GPLL0_STATUS_ACTIVE BIT(17)
> >
> > @@ -77,7 +85,7 @@ static struct vote_clk gcc_blsp1_ahb_clk = {
> >  };
> >
> >  /* SDHCI */
> > -static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
> > +static int apq8016_clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
> This seems like an unrelated change, I don't think we need to namespace
> this function as it's static.

We should follow the same naming convention within a driver to avoid confusion.

[snip]

> > diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
> > index ac4280c6c4c..eaf024a55b0 100644
> > --- a/drivers/serial/serial_msm.c
> > +++ b/drivers/serial/serial_msm.c
> > @@ -248,12 +248,14 @@ static struct msm_serial_data init_serial_data = {
> >  #include <debug_uart.h>
> >
> >  /* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
> Please update the comment to offer some hints about which UART should be
> enabled.

Okay I can add a hint for UART to be board specific.

-Sumit

> > -//int apq8016_clk_init_uart(phys_addr_t gcc_base);
> > +//int apq8016_clk_init_uart1(phys_addr_t gcc_base);
> > +//int apq8016_clk_init_uart2(phys_addr_t gcc_base);
> >
> >  static inline void _debug_uart_init(void)
> >  {
> >       /* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
> > -     //apq8016_clk_init_uart(0x1800000);
> > +     //apq8016_clk_init_uart1(0x1800000);
> > +     //apq8016_clk_init_uart2(0x1800000);
> >       uart_dm_init(&init_serial_data);
> >  }
> >
>
> --
> // Caleb (they/them)
diff mbox series

Patch

diff --git a/drivers/clk/qcom/clock-apq8016.c b/drivers/clk/qcom/clock-apq8016.c
index e6647f7c41d..a620a10a520 100644
--- a/drivers/clk/qcom/clock-apq8016.c
+++ b/drivers/clk/qcom/clock-apq8016.c
@@ -43,6 +43,14 @@ 
 #define BLSP1_UART2_APPS_N		(0x3040)
 #define BLSP1_UART2_APPS_D		(0x3044)
 
+#define BLSP1_UART1_BCR			(0x2038)
+#define BLSP1_UART1_APPS_CBCR		(0x203C)
+#define BLSP1_UART1_APPS_CMD_RCGR	(0x2044)
+#define BLSP1_UART1_APPS_CFG_RCGR	(0x2048)
+#define BLSP1_UART1_APPS_M		(0x204C)
+#define BLSP1_UART1_APPS_N		(0x2050)
+#define BLSP1_UART1_APPS_D		(0x2054)
+
 /* GPLL0 clock control registers */
 #define GPLL0_STATUS_ACTIVE BIT(17)
 
@@ -77,7 +85,7 @@  static struct vote_clk gcc_blsp1_ahb_clk = {
 };
 
 /* SDHCI */
-static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
+static int apq8016_clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
 {
 	int div = 15; /* 100MHz default */
 
@@ -94,6 +102,33 @@  static int clk_init_sdc(struct msm_clk_priv *priv, int slot, uint rate)
 	return rate;
 }
 
+static const struct bcr_regs uart1_regs = {
+	.cfg_rcgr = BLSP1_UART1_APPS_CFG_RCGR,
+	.cmd_rcgr = BLSP1_UART1_APPS_CMD_RCGR,
+	.M = BLSP1_UART1_APPS_M,
+	.N = BLSP1_UART1_APPS_N,
+	.D = BLSP1_UART1_APPS_D,
+};
+
+/* UART: 115200 */
+static int apq8016_clk_init_uart1(phys_addr_t base)
+{
+	/* Enable AHB clock */
+	clk_enable_vote_clk(base, &gcc_blsp1_ahb_clk);
+
+	/* 7372800 uart block clock @ GPLL0 */
+	clk_rcg_set_rate_mnd(base, &uart1_regs, 1, 144, 15625,
+			     CFG_CLK_SRC_GPLL0, 16);
+
+	/* Vote for gpll0 clock */
+	clk_enable_gpll0(base, &gpll0_vote_clk);
+
+	/* Enable core clk */
+	clk_enable_cbc(base + BLSP1_UART1_APPS_CBCR);
+
+	return 0;
+}
+
 static const struct bcr_regs uart2_regs = {
 	.cfg_rcgr = BLSP1_UART2_APPS_CFG_RCGR,
 	.cmd_rcgr = BLSP1_UART2_APPS_CMD_RCGR,
@@ -103,7 +138,7 @@  static const struct bcr_regs uart2_regs = {
 };
 
 /* UART: 115200 */
-int apq8016_clk_init_uart(phys_addr_t base)
+int apq8016_clk_init_uart2(phys_addr_t base)
 {
 	/* Enable AHB clock */
 	clk_enable_vote_clk(base, &gcc_blsp1_ahb_clk);
@@ -127,14 +162,13 @@  static ulong apq8016_clk_set_rate(struct clk *clk, ulong rate)
 
 	switch (clk->id) {
 	case GCC_SDCC1_APPS_CLK: /* SDC1 */
-		return clk_init_sdc(priv, 0, rate);
-		break;
+		return apq8016_clk_init_sdc(priv, 0, rate);
 	case GCC_SDCC2_APPS_CLK: /* SDC2 */
-		return clk_init_sdc(priv, 1, rate);
-		break;
+		return apq8016_clk_init_sdc(priv, 1, rate);
 	case GCC_BLSP1_UART2_APPS_CLK: /* UART2 */
-		return apq8016_clk_init_uart(priv->base);
-		break;
+		return apq8016_clk_init_uart2(priv->base);
+	case GCC_BLSP1_UART1_APPS_CLK: /* UART1 */
+		return apq8016_clk_init_uart1(priv->base);
 	default:
 		return 0;
 	}
diff --git a/drivers/pinctrl/qcom/pinctrl-apq8016.c b/drivers/pinctrl/qcom/pinctrl-apq8016.c
index db0e2124684..cb0e2227079 100644
--- a/drivers/pinctrl/qcom/pinctrl-apq8016.c
+++ b/drivers/pinctrl/qcom/pinctrl-apq8016.c
@@ -29,6 +29,7 @@  static const char * const msm_pinctrl_pins[] = {
 };
 
 static const struct pinctrl_function msm_pinctrl_functions[] = {
+	{"blsp_uart1", 2},
 	{"blsp_uart2", 2},
 };
 
diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
index ac4280c6c4c..eaf024a55b0 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -248,12 +248,14 @@  static struct msm_serial_data init_serial_data = {
 #include <debug_uart.h>
 
 /* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
-//int apq8016_clk_init_uart(phys_addr_t gcc_base);
+//int apq8016_clk_init_uart1(phys_addr_t gcc_base);
+//int apq8016_clk_init_uart2(phys_addr_t gcc_base);
 
 static inline void _debug_uart_init(void)
 {
 	/* Uncomment to turn on UART clocks when debugging U-Boot as aboot on MSM8916 */
-	//apq8016_clk_init_uart(0x1800000);
+	//apq8016_clk_init_uart1(0x1800000);
+	//apq8016_clk_init_uart2(0x1800000);
 	uart_dm_init(&init_serial_data);
 }