Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Vinod,

On 10/3/2018 11:51 AM, Vinod wrote:
Hi Stephen,

Thanks for the comments,

On 01-10-18, 10:19, Stephen Boyd wrote:
Quoting Vinod Koul (2018-09-21 11:59:36)
From: Shefali Jain <shefjain@xxxxxxxxxxxxxx>

Add the clocks supported in global clock controller which clock the
peripherals like BLSPs, SDCC, USB, MDSS etc. Register all the clocks
to the clock framework for the clients to be able to request for them.

Signed-off-by: Shefali Jain <shefjain@xxxxxxxxxxxxxx>
Signed-off-by: Taniya Das <tdas@xxxxxxxxxxxxxx>
Co-developed-by: Taniya Das <tdas@xxxxxxxxxxxxxx>
Signed-off-by: Anu Ramanathan <anur@xxxxxxxxxxxxxx>
[rebase and tidyup for upstream]

Who did the tidying?

both of us :)

Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
---
  - reg : shall contain base register location and length
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 064768699fe7..529d84cc7503 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -235,6 +235,14 @@ config MSM_GCC_8998
           Say Y if you want to use peripheral devices such as UART, SPI,
           i2c, USB, UFS, SD/eMMC, PCIe, etc.
+config QCS_GCC_404
+       tristate "QCS404 Global Clock Controller"
+       depends on COMMON_CLK_QCOM
+       help
+        Support for the global clock controller on QCS404 devices.
+        Say Y if you want to use peripheral devices such as UART, SPI, I2C,
+        USB, SD/eMMC, PCIe, etc.

It seems to include multimedia display clks and ethernet? Maybe include
those too.

Sure will add

+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/clk.h>

Please don't include this.

OK will check if this is required, any reason for not including this?

+/* 930MHz configuration */
+static const struct alpha_pll_config gpll3_config = {
+       .l = 48,
+       .alpha = 0x0,
+       .alpha_en_mask = BIT(24),
+       .post_div_mask = 0xf << 8,
+       .post_div_val = 0x1 << 8,
+       .vco_mask = 0x3 << 20,
+       .main_output_mask = 0x1,
+       .config_ctl_val = 0x4001055b,
+};
+
+static struct pll_vco gpll3_vco[] = {

const?

sure

+static struct clk_branch gcc_pwm1_xo512_clk = {
+       .halt_reg = 0x49004,
+       .halt_check = BRANCH_HALT,
+       .clkr = {
+               .enable_reg = 0x49004,
+               .enable_mask = BIT(0),
+               .hw.init = &(struct clk_init_data){
+                       .name = "gcc_pwm1_xo512_clk",
+                       .ops = &clk_branch2_ops,

Do these pwm clks have a parent clk of the XO?

Yes they do


We do not need to specify the parent here.

+       [GCC_USB_HS_PHY_CFG_AHB_CLK] = &gcc_usb_hs_phy_cfg_ahb_clk.clkr,
+       [GCC_USB_HS_SYSTEM_CLK] = &gcc_usb_hs_system_clk.clkr,
+       [GFX3D_CLK_SRC] = &gfx3d_clk_src.clkr,
+       [GP1_CLK_SRC] = &gp1_clk_src.clkr,

Why are some of these missing GCC_ prefix?

will add..


These clocks in HW plans do not have GCC prefixed, so it better to leave them as they are represented in the HW.

+static int gcc_qcs404_probe(struct platform_device *pdev)
+{
+       struct regmap *regmap;
+       int ret;
+
+       ret = qcom_cc_register_board_clk(&pdev->dev,
+                                        "xo_board", "cxo", 19200000);

You shouldn't need to do this. This function is for transitioning DT
that doesn't have the board clk in DT to something the driver wants to
use, in this case "cxo". So you can either register a fixed factor 1/1
clk to do the translation between board and cxo names, or use xo_board
as the parent of things that can take crystal.

Okay will modify this. If I go about using xo_board as parent, I would
need to register that right? FWIW I see the same thing done in gcc-msm8916


As Stephen suggested it should be defined in DT till we use the clk-smd-rpm.c.


+       if (ret)
+               return ret;
+
+       regmap = qcom_cc_map(pdev, &gcc_qcs404_desc);
+       if (IS_ERR(regmap))
+               return PTR_ERR(regmap);
+
+       clk_alpha_pll_configure(&gpll3_out_main, regmap, &gpll3_config);
+       clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 19200000);

use assigned clock rates from DT please.

ok

+       clk_prepare_enable(apss_ahb_clk_src.clkr.hw.clk);
+       clk_prepare_enable(gpll0_ao_out_main.clkr.hw.clk);

And these should be marked as critical clocks.

Okay and how do we go about doing that?

+#define GCC_USB2A_PHY_SLEEP_CLK                                89
+#define GCC_USB30_MASTER_CLK                           90
+#define GCC_USB30_MOCK_UTMI_CLK                                91
+#define gcc_usb30_sleep_clk                            92
+#define gcc_usb3_phy_aux_clk                           93
+#define gcc_usb3_phy_pipe_clk                          94
+#define gcc_usb_hs_phy_cfg_ahb_clk                     95
+#define gcc_usb_hs_system_clk                          96
+#define gfx3d_clk_src                                  97
+#define gp1_clk_src                                    98
+#define gp2_clk_src                                    99
+#define gp3_clk_src                                    100
+#define gpll0_out_main                                 101
+#define gpll1_out_main                                 102
+#define gpll3_out_main                                 103
+#define gpll4_out_main                                 104
+#define hdmi_app_clk_src                               105
+#define hdmi_pclk_clk_src                              106
+#define mdp_clk_src                                    107
+#define pcie_0_aux_clk_src                             108
+#define pcie_0_pipe_clk_src                            109
+#define pclk0_clk_src                                  110
+#define pdm2_clk_src                                   111
+#define sdcc1_apps_clk_src                             112
+#define sdcc1_ice_core_clk_src                         113
+#define sdcc2_apps_clk_src                             114
+#define usb20_mock_utmi_clk_src                                115
+#define usb30_master_clk_src                           116
+#define usb30_mock_utmi_clk_src                                117
+#define usb3_phy_aux_clk_src                           118
+#define usb_hs_system_clk_src                          119
+#define gpll0_ao_clk_src                               120
+#define wcnss_m_clk                                    121
+#define gcc_usb_hs_inactivity_timers_clk               122

Please capitalize all these macros.

Will do

+#define GCC_GENI_IR_BCR                                        0
+#define GCC_USB_HS_BCR                                 1
+#define GCC_USB2_HS_PHY_ONLY_BCR                       2
+#define GCC_QUSB2_PHY_BCR                              3
+#define GCC_USB_HS_PHY_CFG_AHB_BCR                     4
+#define GCC_USB2A_PHY_BCR                              5
+#define GCC_USB3_PHY_BCR                               6
+#define GCC_USB_30_BCR                                 7
+#define GCC_USB3PHY_PHY_BCR                            8
+#define GCC_PCIE_0_BCR                                 9
+#define GCC_PCIE_0_PHY_BCR                             10
+#define GCC_PCIE_0_LINK_DOWN_BCR                       11
+#define GCC_PCIEPHY_0_PHY_BCR                          12
+#define GCC_EMAC_BCR                                   13

No GDSCs? Ok.

Downstream doesn't seem to have one, will recheck specs.


Downstream uses different way to handle GDSC, there are 2 GDSCs which have to be added 1 for MDSS and 1 OXILI_GX.


--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux