On Mon, 11 Mar 2024 at 17:57, Caleb Connolly <caleb.conno...@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.g...@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)