Re: [PATCH 5/8] clk: qcom: add driver for SM8150 SoC

2024-03-01 Thread Volodymyr Babchuk 


Hi Caleb,

Caleb Connolly  writes:

> On 29/02/2024 14:21, Volodymyr Babchuk wrote:
>> Add clock, reset and power domain driver for SM8150. Driver code is
>> based on the similar U-Boot drivers. All constants are taken from the
>> corresponding Linux driver.
>> 
>> This driver supports clock rate setting only for the debug UART and
>> RGMII/Ethernet modules, because this is all I can test right now.
>> 
>> Signed-off-by: Volodymyr Babchuk 
>> ---
>> 
>>  drivers/clk/qcom/Kconfig|   8 ++
>>  drivers/clk/qcom/Makefile   |   1 +
>>  drivers/clk/qcom/clock-qcom.h   |   1 +
>>  drivers/clk/qcom/clock-sm8150.c | 234 
>>  4 files changed, 244 insertions(+)
>>  create mode 100644 drivers/clk/qcom/clock-sm8150.c
>> 
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index 0df0d1881a..18ccf6a45e 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -47,6 +47,14 @@ config CLK_QCOM_SDM845
>>on the Snapdragon 845 SoC. This driver supports the clocks
>>and resets exposed by the GCC hardware block.
>>  
>> +config CLK_QCOM_SM8150
>> +bool "Qualcomm SM8150 GCC"
>> +select CLK_QCOM
>> +help
>> +  Say Y here to enable support for the Global Clock Controller
>> +  on the Snapdragon 8150 SoC. This driver supports the clocks
>> +  and resets exposed by the GCC hardware block.
>> +
>>  endmenu
>>  
>>  endif
>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index cb179fdac5..12c09ba19e 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -8,3 +8,4 @@ obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o
>>  obj-$(CONFIG_CLK_QCOM_APQ8096) += clock-apq8096.o
>>  obj-$(CONFIG_CLK_QCOM_IPQ4019) += clock-ipq4019.o
>>  obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o
>> +obj-$(CONFIG_CLK_QCOM_SM8150) += clock-sm8150.o
>> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
>> index 12a1eaec2b..41107df216 100644
>> --- a/drivers/clk/qcom/clock-qcom.h
>> +++ b/drivers/clk/qcom/clock-qcom.h
>> @@ -9,6 +9,7 @@
>>  
>>  #define CFG_CLK_SRC_CXO   (0 << 8)
>>  #define CFG_CLK_SRC_GPLL0 (1 << 8)
>> +#define CFG_CLK_SRC_GPLL7_MAIN (3 << 8)
> Please follow the existing scheme and remove the _MAIN suffix.

Ah, yes, planned to do this, but looks like lost it when created a final
version of the patch.

> I'd also be fine with converting all of these definitions to follow how
> the Linux drivers name them.

This should be done in the separate patch, I think.

[...]


>> +static int sm8150_clk_enable(struct clk *clk)
>> +{
>> +struct msm_clk_priv *priv = dev_get_priv(clk->dev);
>> +
>> +clk_enable_gpll0(priv->base, _vote_clk);
> This will write to the GPLL for every single clock, I think you're
> missing a switch case block here.

Yes, you are right.

>> +qcom_gate_clk_en(priv, clk->id);
>> +
>> +return 0;
>> +}
>> +
>> +static const struct qcom_reset_map sm8150_gcc_resets[] = {
>> +[GCC_EMAC_BCR] = { 0x6000 },
>> +[GCC_GPU_BCR] = { 0x71000 },
>> +[GCC_MMSS_BCR] = { 0xb000 },
>> +[GCC_NPU_BCR] = { 0x4d000 },
>> +[GCC_PCIE_0_BCR] = { 0x6b000 },
>> +[GCC_PCIE_0_PHY_BCR] = { 0x6c01c },
>> +[GCC_PCIE_1_BCR] = { 0x8d000 },
>> +[GCC_PCIE_1_PHY_BCR] = { 0x8e01c },
>> +[GCC_PCIE_PHY_BCR] = { 0x6f000 },
>> +[GCC_PDM_BCR] = { 0x33000 },
>> +[GCC_PRNG_BCR] = { 0x34000 },
>> +[GCC_QSPI_BCR] = { 0x24008 },
>> +[GCC_QUPV3_WRAPPER_0_BCR] = { 0x17000 },
>> +[GCC_QUPV3_WRAPPER_1_BCR] = { 0x18000 },
>> +[GCC_QUPV3_WRAPPER_2_BCR] = { 0x1e000 },
>> +[GCC_QUSB2PHY_PRIM_BCR] = { 0x12000 },
>> +[GCC_QUSB2PHY_SEC_BCR] = { 0x12004 },
>> +[GCC_USB3_PHY_PRIM_BCR] = { 0x5 },
>> +[GCC_USB3_DP_PHY_PRIM_BCR] = { 0x50008 },
>> +[GCC_USB3_PHY_SEC_BCR] = { 0x5000c },
>> +[GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
>> +[GCC_SDCC2_BCR] = { 0x14000 },
>> +[GCC_SDCC4_BCR] = { 0x16000 },
>> +[GCC_TSIF_BCR] = { 0x36000 },
>> +[GCC_UFS_CARD_BCR] = { 0x75000 },
>> +[GCC_UFS_PHY_BCR] = { 0x77000 },
>> +[GCC_USB30_PRIM_BCR] = { 0xf000 },
>> +[GCC_USB30_SEC_BCR] = { 0x1 },
>> +[GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
>> +};
>> +
>> +static const struct qcom_power_map sm8150_gcc_power_domains[] = {
> This is really nice! Certainly better than my approach [1].



> Please reorder the patches so that the power domain driver is added
> before this so we don't break future bisects.

power domain driver is added in the previous patch: "[PATCH 4/8] clk:
qcom: add support for power domains uclass"


>> +[EMAC_GDSC] = { 0x6004 },
>> +[PCIE_0_GDSC] = { 0x6b004 },
>> +[PCIE_1_GDSC] = { 0x8d004 },
>> +[UFS_CARD_GDSC] = { 0x75004 },
>> +[UFS_PHY_GDSC] = { 0x7704 },
>> +[USB30_PRIM_GDSC] = { 0x6004 },
>> +[USB30_SEC_GDSC] = { 0x10004 },
>> +};
>> +
>> +
>> +static struct msm_clk_data sm8150_clk_data = {
>> +.resets = sm8150_gcc_resets,

Re: [PATCH 5/8] clk: qcom: add driver for SM8150 SoC

2024-03-01 Thread Caleb Connolly 


>> Please reorder the patches so that the power domain driver is added
>> before this so we don't break future bisects.
> 
> power domain driver is added in the previous patch: "[PATCH 4/8] clk:
> qcom: add support for power domains uclass"

Ah, my mistake!

>>
>> Just in case it's useful to you, I have some work in progress patches
>> for dumping the clock configuration, I hope to upstream these eventually
>> but in the mean time you can find them here:
> 
> This is what bothers me now. Looks like I jumped in the middle of your
> USB series and Sumit's Qulacom device tree binding series. I don't want
> to interfere with the ongoing work, so probably I postpone posting my v2
> unless at least Sumit's series are taken into the mainline. On other
> hand, there are couple of patches that are not directly related to the
> SM8155P-ADP, like my fix to the clock driver or already mentioned "clk:
> qcom: add support for power domains uclass", which you can reuse. So I
> can post them separately or you can include them into your series if you
> want. What is your opinion?

The big conflict is really just the board support stuff and DTS. The
other patches would definitely be good to have.

Could you rebase this series on top of qcom-next [1]? The big series has
gone in now (and should be in U-Boot next shortly) so we shouldn't have
to delay yours.

Your last patch introducing the board support will conflict, but you
don't have any heavily board specific features so it should be pretty easy:

* Drop init_sm8150.c and sysmap-sm8150.c (everything they do is handled
generically in board.c)
* Drop your changes to mach-snapdragon/Kconfig
* Delete board/qualcomm/sa8155p-adp/Kconfig
* Replace CONFIG_TARGET_SA8155P_ADP=y with
CONFIG_SYS_BOARD="sa8155p-adp" in your defconfig
* Set CONFIG_SYS_CONFIG_NAME="sa8155p_adp" (or rename the config file to
use a '-' instead).

It's now possible to use pre-processor directives from defconfigs, so
you could also rework your defconfig to #include "qcom_defconfig" and
that way only have to define your board specific options, but I guess
there isn't much room in the hyp partition so you might not want all the
bells and whistles - totally up to you.

If you could split this into series, one with your fixes and drivers,
and another adding the DTS and board changes I think that would be fine.

Regarding the DT stuff, you probably drop your sm8150 DTS patch and
instead select OF_UPSTREAM for ARCH_SNAPDRAGON, I'll likely send a patch
for this soon.

[1]: https://source.denx.de/u-boot/custodians/u-boot-snapdragon
> 

-- 
// Caleb (they/them)

Re: [PATCH 5/8] clk: qcom: add driver for SM8150 SoC

2024-03-01 Thread Caleb Connolly 



On 29/02/2024 14:21, Volodymyr Babchuk wrote:
> Add clock, reset and power domain driver for SM8150. Driver code is
> based on the similar U-Boot drivers. All constants are taken from the
> corresponding Linux driver.
> 
> This driver supports clock rate setting only for the debug UART and
> RGMII/Ethernet modules, because this is all I can test right now.
> 
> Signed-off-by: Volodymyr Babchuk 
> ---
> 
>  drivers/clk/qcom/Kconfig|   8 ++
>  drivers/clk/qcom/Makefile   |   1 +
>  drivers/clk/qcom/clock-qcom.h   |   1 +
>  drivers/clk/qcom/clock-sm8150.c | 234 
>  4 files changed, 244 insertions(+)
>  create mode 100644 drivers/clk/qcom/clock-sm8150.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 0df0d1881a..18ccf6a45e 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -47,6 +47,14 @@ config CLK_QCOM_SDM845
> on the Snapdragon 845 SoC. This driver supports the clocks
> and resets exposed by the GCC hardware block.
>  
> +config CLK_QCOM_SM8150
> + bool "Qualcomm SM8150 GCC"
> + select CLK_QCOM
> + help
> +   Say Y here to enable support for the Global Clock Controller
> +   on the Snapdragon 8150 SoC. This driver supports the clocks
> +   and resets exposed by the GCC hardware block.
> +
>  endmenu
>  
>  endif
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index cb179fdac5..12c09ba19e 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o
>  obj-$(CONFIG_CLK_QCOM_APQ8096) += clock-apq8096.o
>  obj-$(CONFIG_CLK_QCOM_IPQ4019) += clock-ipq4019.o
>  obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o
> +obj-$(CONFIG_CLK_QCOM_SM8150) += clock-sm8150.o
> diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
> index 12a1eaec2b..41107df216 100644
> --- a/drivers/clk/qcom/clock-qcom.h
> +++ b/drivers/clk/qcom/clock-qcom.h
> @@ -9,6 +9,7 @@
>  
>  #define CFG_CLK_SRC_CXO   (0 << 8)
>  #define CFG_CLK_SRC_GPLL0 (1 << 8)
> +#define CFG_CLK_SRC_GPLL7_MAIN (3 << 8)
Please follow the existing scheme and remove the _MAIN suffix.
I'd also be fine with converting all of these definitions to follow how
the Linux drivers name them.

>  #define CFG_CLK_SRC_GPLL0_EVEN (6 << 8)
>  #define CFG_CLK_SRC_MASK  (7 << 8)
>  
> diff --git a/drivers/clk/qcom/clock-sm8150.c b/drivers/clk/qcom/clock-sm8150.c
> new file mode 100644
> index 00..e23dbccdd3
> --- /dev/null
> +++ b/drivers/clk/qcom/clock-sm8150.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Clock drivers for Qualcomm SM8150
> + *
> + * Volodymyr Babchuk 
> + * Copyright (c) 2024 EPAM Systems.
> + *
> + * Based on U-Boot driver for SDM845. Constants are taken from the Linux 
> driver.
> + */
> +
> +#include 
> +#include 
> +#include 
This header is unused
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "clock-qcom.h"
> +
> +static struct pll_vote_clk gpll7_vote_clk = {
> + .status = 0x1a000,
> + .status_bit = BIT(31),
> + .ena_vote = 0x52000,
> + .vote_bit = BIT(7),
> +};
> +
> +static const struct freq_tbl ftbl_gcc_qupv3_wrap0_s0_clk_src[] = {
> + F(7372800, CFG_CLK_SRC_GPLL0_EVEN, 1, 384, 15625),
> + F(14745600, CFG_CLK_SRC_GPLL0_EVEN, 1, 768, 15625),
> + F(1920, CFG_CLK_SRC_CXO, 1, 0, 0),
> + F(29491200, CFG_CLK_SRC_GPLL0_EVEN, 1, 1536, 15625),
> + F(3200, CFG_CLK_SRC_GPLL0_EVEN, 1, 8, 75),
> + F(4800, CFG_CLK_SRC_GPLL0_EVEN, 1, 4, 25),
> + F(6400, CFG_CLK_SRC_GPLL0_EVEN, 1, 16, 75),
> + F(8000, CFG_CLK_SRC_GPLL0_EVEN, 1, 4, 15),
> + F(9600, CFG_CLK_SRC_GPLL0_EVEN, 1, 8, 25),
> + F(1, CFG_CLK_SRC_GPLL0_EVEN, 3, 0, 0),
> + F(10240, CFG_CLK_SRC_GPLL0_EVEN, 1, 128, 375),
> + F(11200, CFG_CLK_SRC_GPLL0_EVEN, 1, 28, 75),
> + F(117964800, CFG_CLK_SRC_GPLL0_EVEN, 1, 6144, 15625),
> + F(12000, CFG_CLK_SRC_GPLL0_EVEN, 2.5, 0, 0),
> + F(12800, CFG_CLK_SRC_GPLL0, 1, 16, 75),
> + { }
> +};
> +
> +static const struct freq_tbl ftbl_gcc_emac_rgmii_clk_src[] = {
> + F(250, CFG_CLK_SRC_CXO, 1, 25, 192),
> + F(500, CFG_CLK_SRC_CXO, 1, 25, 96),
> + F(1920, CFG_CLK_SRC_CXO, 1, 0, 0),
> + F(2500, CFG_CLK_SRC_GPLL0_EVEN, 12, 0, 0),
> + F(5000, CFG_CLK_SRC_GPLL0_EVEN, 6, 0, 0),
> + F(12500, CFG_CLK_SRC_GPLL7_MAIN, 4, 0, 0),
> + F(25000, CFG_CLK_SRC_GPLL7_MAIN, 2, 0, 0),
> + { }
> +};
> +
> +static const struct bcr_regs uart2_regs = {
> + .cfg_rcgr = 0x1814C,
> + .cmd_rcgr = 0x18148,
> + .M = 0x18150,
> + .N = 0x18154,
> + .D = 0x18158,
> +};
> +
> +static const struct bcr_regs rgmii_regs = {
> + .cfg_rcgr = 0x6020,
> + .cmd_rcgr = 0x601C,
> + .M = 0x6024,
> + .N = 0x6028,
> + .D = 0x602C,
> +};
> +
> +static ulong sm8150_clk_set_rate(struct clk

[PATCH 5/8] clk: qcom: add driver for SM8150 SoC

2024-02-29 Thread Volodymyr Babchuk 
Add clock, reset and power domain driver for SM8150. Driver code is
based on the similar U-Boot drivers. All constants are taken from the
corresponding Linux driver.

This driver supports clock rate setting only for the debug UART and
RGMII/Ethernet modules, because this is all I can test right now.

Signed-off-by: Volodymyr Babchuk 
---

 drivers/clk/qcom/Kconfig|   8 ++
 drivers/clk/qcom/Makefile   |   1 +
 drivers/clk/qcom/clock-qcom.h   |   1 +
 drivers/clk/qcom/clock-sm8150.c | 234 
 4 files changed, 244 insertions(+)
 create mode 100644 drivers/clk/qcom/clock-sm8150.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 0df0d1881a..18ccf6a45e 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -47,6 +47,14 @@ config CLK_QCOM_SDM845
  on the Snapdragon 845 SoC. This driver supports the clocks
  and resets exposed by the GCC hardware block.
 
+config CLK_QCOM_SM8150
+   bool "Qualcomm SM8150 GCC"
+   select CLK_QCOM
+   help
+ Say Y here to enable support for the Global Clock Controller
+ on the Snapdragon 8150 SoC. This driver supports the clocks
+ and resets exposed by the GCC hardware block.
+
 endmenu
 
 endif
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index cb179fdac5..12c09ba19e 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_CLK_QCOM_APQ8016) += clock-apq8016.o
 obj-$(CONFIG_CLK_QCOM_APQ8096) += clock-apq8096.o
 obj-$(CONFIG_CLK_QCOM_IPQ4019) += clock-ipq4019.o
 obj-$(CONFIG_CLK_QCOM_QCS404) += clock-qcs404.o
+obj-$(CONFIG_CLK_QCOM_SM8150) += clock-sm8150.o
diff --git a/drivers/clk/qcom/clock-qcom.h b/drivers/clk/qcom/clock-qcom.h
index 12a1eaec2b..41107df216 100644
--- a/drivers/clk/qcom/clock-qcom.h
+++ b/drivers/clk/qcom/clock-qcom.h
@@ -9,6 +9,7 @@
 
 #define CFG_CLK_SRC_CXO   (0 << 8)
 #define CFG_CLK_SRC_GPLL0 (1 << 8)
+#define CFG_CLK_SRC_GPLL7_MAIN (3 << 8)
 #define CFG_CLK_SRC_GPLL0_EVEN (6 << 8)
 #define CFG_CLK_SRC_MASK  (7 << 8)
 
diff --git a/drivers/clk/qcom/clock-sm8150.c b/drivers/clk/qcom/clock-sm8150.c
new file mode 100644
index 00..e23dbccdd3
--- /dev/null
+++ b/drivers/clk/qcom/clock-sm8150.c
@@ -0,0 +1,234 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Clock drivers for Qualcomm SM8150
+ *
+ * Volodymyr Babchuk 
+ * Copyright (c) 2024 EPAM Systems.
+ *
+ * Based on U-Boot driver for SDM845. Constants are taken from the Linux 
driver.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "clock-qcom.h"
+
+static struct pll_vote_clk gpll7_vote_clk = {
+   .status = 0x1a000,
+   .status_bit = BIT(31),
+   .ena_vote = 0x52000,
+   .vote_bit = BIT(7),
+};
+
+static const struct freq_tbl ftbl_gcc_qupv3_wrap0_s0_clk_src[] = {
+   F(7372800, CFG_CLK_SRC_GPLL0_EVEN, 1, 384, 15625),
+   F(14745600, CFG_CLK_SRC_GPLL0_EVEN, 1, 768, 15625),
+   F(1920, CFG_CLK_SRC_CXO, 1, 0, 0),
+   F(29491200, CFG_CLK_SRC_GPLL0_EVEN, 1, 1536, 15625),
+   F(3200, CFG_CLK_SRC_GPLL0_EVEN, 1, 8, 75),
+   F(4800, CFG_CLK_SRC_GPLL0_EVEN, 1, 4, 25),
+   F(6400, CFG_CLK_SRC_GPLL0_EVEN, 1, 16, 75),
+   F(8000, CFG_CLK_SRC_GPLL0_EVEN, 1, 4, 15),
+   F(9600, CFG_CLK_SRC_GPLL0_EVEN, 1, 8, 25),
+   F(1, CFG_CLK_SRC_GPLL0_EVEN, 3, 0, 0),
+   F(10240, CFG_CLK_SRC_GPLL0_EVEN, 1, 128, 375),
+   F(11200, CFG_CLK_SRC_GPLL0_EVEN, 1, 28, 75),
+   F(117964800, CFG_CLK_SRC_GPLL0_EVEN, 1, 6144, 15625),
+   F(12000, CFG_CLK_SRC_GPLL0_EVEN, 2.5, 0, 0),
+   F(12800, CFG_CLK_SRC_GPLL0, 1, 16, 75),
+   { }
+};
+
+static const struct freq_tbl ftbl_gcc_emac_rgmii_clk_src[] = {
+   F(250, CFG_CLK_SRC_CXO, 1, 25, 192),
+   F(500, CFG_CLK_SRC_CXO, 1, 25, 96),
+   F(1920, CFG_CLK_SRC_CXO, 1, 0, 0),
+   F(2500, CFG_CLK_SRC_GPLL0_EVEN, 12, 0, 0),
+   F(5000, CFG_CLK_SRC_GPLL0_EVEN, 6, 0, 0),
+   F(12500, CFG_CLK_SRC_GPLL7_MAIN, 4, 0, 0),
+   F(25000, CFG_CLK_SRC_GPLL7_MAIN, 2, 0, 0),
+   { }
+};
+
+static const struct bcr_regs uart2_regs = {
+   .cfg_rcgr = 0x1814C,
+   .cmd_rcgr = 0x18148,
+   .M = 0x18150,
+   .N = 0x18154,
+   .D = 0x18158,
+};
+
+static const struct bcr_regs rgmii_regs = {
+   .cfg_rcgr = 0x6020,
+   .cmd_rcgr = 0x601C,
+   .M = 0x6024,
+   .N = 0x6028,
+   .D = 0x602C,
+};
+
+static ulong sm8150_clk_set_rate(struct clk *clk, ulong rate)
+{
+   struct msm_clk_priv *priv = dev_get_priv(clk->dev);
+   const struct freq_tbl *freq;
+
+   switch (clk->id) {
+   case GCC_QUPV3_WRAP1_S4_CLK: /* UART2 aka debug-uart */
+   freq = qcom_find_freq(ftbl_gcc_qupv3_wrap0_s0_clk_src, rate);
+   clk_rcg_set_rate_mnd(priv->base, _regs,
+freq->pre_div, freq->m, freq->n,