diff mbox series

[1/2] clk: qcom: Export clk_alpha_pll_configure()

Message ID 20180921185936.9590-1-vkoul@kernel.org
State New
Headers show
Series [1/2] clk: qcom: Export clk_alpha_pll_configure() | expand

Commit Message

Vinod Koul Sept. 21, 2018, 6:59 p.m. UTC
From: Bjorn Andersson <bjorn.andersson@linaro.org>


This is used by the QCS404 GCC driver, export it to allow that driver to
be compiled as a module..

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Signed-off-by: Vinod Koul <vkoul@kernel.org>

---
 drivers/clk/qcom/clk-alpha-pll.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.14.4

Comments

Vinod Koul Oct. 3, 2018, 6:21 a.m. UTC | #1
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@codeaurora.org>

> > 

> > 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@codeaurora.org>

> > Signed-off-by: Taniya Das <tdas@codeaurora.org>

> > Co-developed-by: Taniya Das <tdas@codeaurora.org>

> > Signed-off-by: Anu Ramanathan <anur@codeaurora.org>

> > [rebase and tidyup for upstream]

> 

> Who did the tidying?


both of us :)

> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> > ---

> >  - 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

> > +       [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..

> > +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

> 

> > +       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.

-- 
~Vinod
Vinod Koul Oct. 7, 2018, 1:31 p.m. UTC | #2
Hi Taniya,

Thanks for the review, It would be great if you can strip the irrelevant
context while replying, makes it easier for people to follow.

On 06-10-18, 23:28, Taniya Das wrote:

> > +static struct clk_rcg2 pclk0_clk_src = {

> > +	.cmd_rcgr = 0x4d000,

> > +	.mnd_width = 8,

> > +	.hid_width = 5,

> > +	.parent_map = gcc_parent_map_12,

> > +	.clkr.hw.init = &(struct clk_init_data){

> > +		.name = "pclk0_clk_src",

> > +		.parent_names = gcc_parent_names_12,

> > +		.num_parents = 4,

> > +		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,

> 

> Please remove the NOCACHE flag for all display RCGs as there are no real

> requirement.


Thanks for the suggestion, will do

> > +++ b/include/dt-bindings/clock/qcom,gcc-qcs404.h

> > @@ -0,0 +1,166 @@

> > +/* SPDX-License-Identifier: GPL-2.0 */

> > +/*

> > + * Copyright (c) 2018, The Linux Foundation. All rights reserved.

> > + */

> > +

> > +#ifndef _DT_BINDINGS_CLK_QCOM_GCC_QCS404_H

> > +#define _DT_BINDINGS_CLK_QCOM_GCC_QCS404_H

> > +

> > +#define APSS_AHB_CLK_SRC				0

> > +#define BLSP1_QUP0_I2C_APPS_CLK_SRC			1

> > +#define BLSP1_QUP0_SPI_APPS_CLK_SRC			2

> > +#define BLSP1_QUP1_I2C_APPS_CLK_SRC			3

> > +#define BLSP1_QUP1_SPI_APPS_CLK_SRC			4

> > +#define BLSP1_QUP2_I2C_APPS_CLK_SRC			5

> > +#define BLSP1_QUP2_SPI_APPS_CLK_SRC			6

> > +#define BLSP1_QUP3_I2C_APPS_CLK_SRC			7

> > +#define BLSP1_QUP3_SPI_APPS_CLK_SRC			8

> > +#define BLSP1_QUP4_I2C_APPS_CLK_SRC			9

> > +#define BLSP1_QUP4_SPI_APPS_CLK_SRC			10

> > +#define BLSP1_UART0_APPS_CLK_SRC			11

> > +#define BLSP1_UART1_APPS_CLK_SRC			12

> > +#define BLSP1_UART2_APPS_CLK_SRC			13

> > +#define BLSP1_UART3_APPS_CLK_SRC			14

> > +#define BLSP2_QUP0_I2C_APPS_CLK_SRC			15

> > +#define BLSP2_QUP0_SPI_APPS_CLK_SRC			16

> > +#define BLSP2_UART0_APPS_CLK_SRC			17

> > +#define BYTE0_CLK_SRC					18

> > +#define EMAC_CLK_SRC					19

> > +#define EMAC_PTP_CLK_SRC				20

> > +#define ESC0_CLK_SRC					21

> > +#define GCC_APSS_AHB_CLK				22

> > +#define GCC_APSS_AXI_CLK				23

> > +#define GCC_BIMC_APSS_AXI_CLK				24

> > +#define GCC_BIMC_GFX_CLK				25

> > +#define GCC_BIMC_MDSS_CLK				26

> > +#define GCC_BLSP1_AHB_CLK				27

> > +#define GCC_BLSP1_QUP0_I2C_APPS_CLK			28

> > +#define GCC_BLSP1_QUP0_SPI_APPS_CLK			29

> > +#define GCC_BLSP1_QUP1_I2C_APPS_CLK			30

> > +#define GCC_BLSP1_QUP1_SPI_APPS_CLK			31

> > +#define GCC_BLSP1_QUP2_I2C_APPS_CLK			32

> > +#define GCC_BLSP1_QUP2_SPI_APPS_CLK			33

> > +#define GCC_BLSP1_QUP3_I2C_APPS_CLK			34

> > +#define GCC_BLSP1_QUP3_SPI_APPS_CLK			35

> > +#define GCC_BLSP1_QUP4_I2C_APPS_CLK			36

> > +#define GCC_BLSP1_QUP4_SPI_APPS_CLK			37

> > +#define GCC_BLSP1_UART0_APPS_CLK			38

> > +#define GCC_BLSP1_UART1_APPS_CLK			39

> > +#define GCC_BLSP1_UART2_APPS_CLK			40

> > +#define GCC_BLSP1_UART3_APPS_CLK			41

> > +#define GCC_BLSP2_AHB_CLK				42

> > +#define GCC_BLSP2_QUP0_I2C_APPS_CLK			43

> > +#define GCC_BLSP2_QUP0_SPI_APPS_CLK			44

> > +#define GCC_BLSP2_UART0_APPS_CLK			45

> > +#define GCC_BOOT_ROM_AHB_CLK				46

> > +#define GCC_DCC_CLK					47

> > +#define GCC_GENI_IR_H_CLK				48

> > +#define GCC_ETH_AXI_CLK					49

> > +#define GCC_ETH_PTP_CLK					50

> > +#define GCC_ETH_RGMII_CLK				51

> > +#define GCC_ETH_SLAVE_AHB_CLK				52

> > +#define GCC_GENI_IR_S_CLK				53

> > +#define GCC_GP1_CLK					54

> > +#define GCC_GP2_CLK					55

> > +#define GCC_GP3_CLK					56

> > +#define GCC_MDSS_AHB_CLK				57

> > +#define GCC_MDSS_AXI_CLK				58

> > +#define GCC_MDSS_BYTE0_CLK				59

> > +#define GCC_MDSS_ESC0_CLK				60

> > +#define GCC_MDSS_HDMI_APP_CLK				61

> > +#define GCC_MDSS_HDMI_PCLK_CLK				62

> > +#define GCC_MDSS_MDP_CLK				63

> > +#define GCC_MDSS_PCLK0_CLK				64

> > +#define GCC_MDSS_VSYNC_CLK				65

> > +#define GCC_OXILI_AHB_CLK				66

> > +#define GCC_OXILI_GFX3D_CLK				67

> > +#define GCC_PCIE_0_AUX_CLK				68

> > +#define GCC_PCIE_0_CFG_AHB_CLK				69

> > +#define GCC_PCIE_0_MSTR_AXI_CLK				70

> > +#define GCC_PCIE_0_PIPE_CLK				71

> > +#define GCC_PCIE_0_SLV_AXI_CLK				72

> > +#define GCC_PCNOC_USB2_CLK				73

> > +#define GCC_PCNOC_USB3_CLK				74

> > +#define GCC_PDM2_CLK					75

> > +#define GCC_PDM_AHB_CLK					76

> > +#define VSYNC_CLK_SRC					77

> > +#define GCC_PRNG_AHB_CLK				78

> > +#define GCC_PWM0_XO512_CLK				79

> > +#define GCC_PWM1_XO512_CLK				80

> > +#define GCC_PWM2_XO512_CLK				81

> > +#define GCC_SDCC1_AHB_CLK				82

> > +#define GCC_SDCC1_APPS_CLK				83

> > +#define GCC_SDCC1_ICE_CORE_CLK				84

> > +#define GCC_SDCC2_AHB_CLK				85

> > +#define GCC_SDCC2_APPS_CLK				86

> > +#define GCC_SYS_NOC_USB3_CLK				87

> > +#define GCC_USB20_MOCK_UTMI_CLK				88

> > +#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

> 

> Please remove WCNSS_M_CLK.


And the reason for that?

-- 
~Vinod
Vinod Koul Oct. 8, 2018, 3:51 a.m. UTC | #3
On 07-10-18, 19:38, Stephen Boyd wrote:
> Quoting Vinod (2018-10-02 23:21:03)

> > 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@codeaurora.org>

> > > > 

> > > > 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@codeaurora.org>

> > > > Signed-off-by: Taniya Das <tdas@codeaurora.org>

> > > > Co-developed-by: Taniya Das <tdas@codeaurora.org>

> > > > Signed-off-by: Anu Ramanathan <anur@codeaurora.org>

> > > > [rebase and tidyup for upstream]

> > > 

> > > Who did the tidying?

> > 

> > both of us :)

> 

> OK, please add the username of both people per the kernel sign off

> standards.

> 

> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>


Sorry not sure I understand, Bjorn and me did cleanup and we signed-off
per process, did I miss something?

> > > > +#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?

> 

> So we can easily split clk consumers and clk providers.


That was my thought, thanks for confirming :)

> > > > +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

> 

> Cool, we should add them or add a comment explaining why they don't have

> parents listed here.


ok

> > > > +       [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..

> 

> Thanks!


Btw Taniya also commented on this, do you want this as GCC_ or as per hw
documentation?

> > > > +       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?

> 

> You mark the clk flags with CLK_IS_CRITICAL.


Thanks will do.


-- 
~Vinod
Taniya Das Oct. 8, 2018, 6:28 a.m. UTC | #4
On 10/7/2018 7:01 PM, Vinod wrote:
> Hi Taniya,

> 

> Thanks for the review, It would be great if you can strip the irrelevant

> context while replying, makes it easier for people to follow.

> 

> On 06-10-18, 23:28, Taniya Das wrote:

> 

>>> +static struct clk_rcg2 pclk0_clk_src = {

>>> +	.cmd_rcgr = 0x4d000,

>>> +	.mnd_width = 8,

>>> +	.hid_width = 5,

>>> +	.parent_map = gcc_parent_map_12,

>>> +	.clkr.hw.init = &(struct clk_init_data){

>>> +		.name = "pclk0_clk_src",

>>> +		.parent_names = gcc_parent_names_12,

>>> +		.num_parents = 4,

>>> +		.flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,

>>

>> Please remove the NOCACHE flag for all display RCGs as there are no real

>> requirement.

> 

> Thanks for the suggestion, will do

> 

>>> +++ b/include/dt-bindings/clock/qcom,gcc-qcs404.h

>>> @@ -0,0 +1,166 @@

>>> +/* SPDX-License-Identifier: GPL-2.0 */

>>> +/*

>>> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.

>>> + */

>>> +

>>> +#ifndef _DT_BINDINGS_CLK_QCOM_GCC_QCS404_H

>>> +#define _DT_BINDINGS_CLK_QCOM_GCC_QCS404_H

>>> +

>>> +#define APSS_AHB_CLK_SRC				0

>>> +#define BLSP1_QUP0_I2C_APPS_CLK_SRC			1

>>> +#define BLSP1_QUP0_SPI_APPS_CLK_SRC			2

>>> +#define BLSP1_QUP1_I2C_APPS_CLK_SRC			3

>>> +#define BLSP1_QUP1_SPI_APPS_CLK_SRC			4

>>> +#define BLSP1_QUP2_I2C_APPS_CLK_SRC			5

>>> +#define BLSP1_QUP2_SPI_APPS_CLK_SRC			6

>>> +#define BLSP1_QUP3_I2C_APPS_CLK_SRC			7

>>> +#define BLSP1_QUP3_SPI_APPS_CLK_SRC			8

>>> +#define BLSP1_QUP4_I2C_APPS_CLK_SRC			9

>>> +#define BLSP1_QUP4_SPI_APPS_CLK_SRC			10

>>> +#define BLSP1_UART0_APPS_CLK_SRC			11

>>> +#define BLSP1_UART1_APPS_CLK_SRC			12

>>> +#define BLSP1_UART2_APPS_CLK_SRC			13

>>> +#define BLSP1_UART3_APPS_CLK_SRC			14

>>> +#define BLSP2_QUP0_I2C_APPS_CLK_SRC			15

>>> +#define BLSP2_QUP0_SPI_APPS_CLK_SRC			16

>>> +#define BLSP2_UART0_APPS_CLK_SRC			17

>>> +#define BYTE0_CLK_SRC					18

>>> +#define EMAC_CLK_SRC					19

>>> +#define EMAC_PTP_CLK_SRC				20

>>> +#define ESC0_CLK_SRC					21

>>> +#define GCC_APSS_AHB_CLK				22

>>> +#define GCC_APSS_AXI_CLK				23

>>> +#define GCC_BIMC_APSS_AXI_CLK				24

>>> +#define GCC_BIMC_GFX_CLK				25

>>> +#define GCC_BIMC_MDSS_CLK				26

>>> +#define GCC_BLSP1_AHB_CLK				27

>>> +#define GCC_BLSP1_QUP0_I2C_APPS_CLK			28

>>> +#define GCC_BLSP1_QUP0_SPI_APPS_CLK			29

>>> +#define GCC_BLSP1_QUP1_I2C_APPS_CLK			30

>>> +#define GCC_BLSP1_QUP1_SPI_APPS_CLK			31

>>> +#define GCC_BLSP1_QUP2_I2C_APPS_CLK			32

>>> +#define GCC_BLSP1_QUP2_SPI_APPS_CLK			33

>>> +#define GCC_BLSP1_QUP3_I2C_APPS_CLK			34

>>> +#define GCC_BLSP1_QUP3_SPI_APPS_CLK			35

>>> +#define GCC_BLSP1_QUP4_I2C_APPS_CLK			36

>>> +#define GCC_BLSP1_QUP4_SPI_APPS_CLK			37

>>> +#define GCC_BLSP1_UART0_APPS_CLK			38

>>> +#define GCC_BLSP1_UART1_APPS_CLK			39

>>> +#define GCC_BLSP1_UART2_APPS_CLK			40

>>> +#define GCC_BLSP1_UART3_APPS_CLK			41

>>> +#define GCC_BLSP2_AHB_CLK				42

>>> +#define GCC_BLSP2_QUP0_I2C_APPS_CLK			43

>>> +#define GCC_BLSP2_QUP0_SPI_APPS_CLK			44

>>> +#define GCC_BLSP2_UART0_APPS_CLK			45

>>> +#define GCC_BOOT_ROM_AHB_CLK				46

>>> +#define GCC_DCC_CLK					47

>>> +#define GCC_GENI_IR_H_CLK				48

>>> +#define GCC_ETH_AXI_CLK					49

>>> +#define GCC_ETH_PTP_CLK					50

>>> +#define GCC_ETH_RGMII_CLK				51

>>> +#define GCC_ETH_SLAVE_AHB_CLK				52

>>> +#define GCC_GENI_IR_S_CLK				53

>>> +#define GCC_GP1_CLK					54

>>> +#define GCC_GP2_CLK					55

>>> +#define GCC_GP3_CLK					56

>>> +#define GCC_MDSS_AHB_CLK				57

>>> +#define GCC_MDSS_AXI_CLK				58

>>> +#define GCC_MDSS_BYTE0_CLK				59

>>> +#define GCC_MDSS_ESC0_CLK				60

>>> +#define GCC_MDSS_HDMI_APP_CLK				61

>>> +#define GCC_MDSS_HDMI_PCLK_CLK				62

>>> +#define GCC_MDSS_MDP_CLK				63

>>> +#define GCC_MDSS_PCLK0_CLK				64

>>> +#define GCC_MDSS_VSYNC_CLK				65

>>> +#define GCC_OXILI_AHB_CLK				66

>>> +#define GCC_OXILI_GFX3D_CLK				67

>>> +#define GCC_PCIE_0_AUX_CLK				68

>>> +#define GCC_PCIE_0_CFG_AHB_CLK				69

>>> +#define GCC_PCIE_0_MSTR_AXI_CLK				70

>>> +#define GCC_PCIE_0_PIPE_CLK				71

>>> +#define GCC_PCIE_0_SLV_AXI_CLK				72

>>> +#define GCC_PCNOC_USB2_CLK				73

>>> +#define GCC_PCNOC_USB3_CLK				74

>>> +#define GCC_PDM2_CLK					75

>>> +#define GCC_PDM_AHB_CLK					76

>>> +#define VSYNC_CLK_SRC					77

>>> +#define GCC_PRNG_AHB_CLK				78

>>> +#define GCC_PWM0_XO512_CLK				79

>>> +#define GCC_PWM1_XO512_CLK				80

>>> +#define GCC_PWM2_XO512_CLK				81

>>> +#define GCC_SDCC1_AHB_CLK				82

>>> +#define GCC_SDCC1_APPS_CLK				83

>>> +#define GCC_SDCC1_ICE_CORE_CLK				84

>>> +#define GCC_SDCC2_AHB_CLK				85

>>> +#define GCC_SDCC2_APPS_CLK				86

>>> +#define GCC_SYS_NOC_USB3_CLK				87

>>> +#define GCC_USB20_MOCK_UTMI_CLK				88

>>> +#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

>>

>> Please remove WCNSS_M_CLK.

> 

> And the reason for that?

> 


This is a dummy/fixed clock and not a real clock used by clients.

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

--
Stephen Boyd Oct. 11, 2018, 7:19 a.m. UTC | #5
Quoting Vinod (2018-10-07 20:51:44)
> On 07-10-18, 19:38, Stephen Boyd wrote:

> > Quoting Vinod (2018-10-02 23:21:03)

> > > 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@codeaurora.org>

> > > > > 

> > > > > 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@codeaurora.org>

> > > > > Signed-off-by: Taniya Das <tdas@codeaurora.org>

> > > > > Co-developed-by: Taniya Das <tdas@codeaurora.org>

> > > > > Signed-off-by: Anu Ramanathan <anur@codeaurora.org>

> > > > > [rebase and tidyup for upstream]

> > > > 

> > > > Who did the tidying?

> > > 

> > > both of us :)

> > 

> > OK, please add the username of both people per the kernel sign off

> > standards.

> > 

> > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> 

> Sorry not sure I understand, Bjorn and me did cleanup and we signed-off

> per process, did I miss something?


I mean doing something like:

[bjorn.andersson@linaro.org: Clean and tidy]
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

[vkoul@kernel.org: Clean and tidy even more]
Signed-off-by: Vinod Koul <vkoul@kernel.org>


Would be the kernel standard for maintainer tags.

> 

> > > > > +       [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..

> > 

> > Thanks!

> 

> Btw Taniya also commented on this, do you want this as GCC_ or as per hw

> documentation?

> 


I don't care. Either way is fine with me.
Vinod Koul Oct. 11, 2018, 9:32 a.m. UTC | #6
On 11-10-18, 00:19, Stephen Boyd wrote:
> Quoting Vinod (2018-10-07 20:51:44)

> > On 07-10-18, 19:38, Stephen Boyd wrote:

> > > Quoting Vinod (2018-10-02 23:21:03)

> > > > 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@codeaurora.org>

> > > > > > 

> > > > > > 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@codeaurora.org>

> > > > > > Signed-off-by: Taniya Das <tdas@codeaurora.org>

> > > > > > Co-developed-by: Taniya Das <tdas@codeaurora.org>

> > > > > > Signed-off-by: Anu Ramanathan <anur@codeaurora.org>

> > > > > > [rebase and tidyup for upstream]

> > > > > 

> > > > > Who did the tidying?

> > > > 

> > > > both of us :)

> > > 

> > > OK, please add the username of both people per the kernel sign off

> > > standards.

> > > 

> > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> > > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> > 

> > Sorry not sure I understand, Bjorn and me did cleanup and we signed-off

> > per process, did I miss something?

> 

> I mean doing something like:

> 

> [bjorn.andersson@linaro.org: Clean and tidy]

> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

> [vkoul@kernel.org: Clean and tidy even more]

> Signed-off-by: Vinod Koul <vkoul@kernel.org>

> 

> Would be the kernel standard for maintainer tags.


Ah I did:

[bamse, vkoul: rebase and tidyup for upstream]

I can wait for comments if you have for v2 and update and send v3
then?

> 

> > 

> > > > > > +       [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..

> > > 

> > > Thanks!

> > 

> > Btw Taniya also commented on this, do you want this as GCC_ or as per hw

> > documentation?

> 

> I don't care. Either way is fine with me.


I have used GCC_ :)

-- 
~Vinod
diff mbox series

Patch

diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index a91d97cecbad..0ced4a5a9a17 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -220,6 +220,7 @@  void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
 	if (pll->flags & SUPPORTS_FSM_MODE)
 		qcom_pll_set_fsm_mode(regmap, PLL_MODE(pll), 6, 0);
 }
+EXPORT_SYMBOL_GPL(clk_alpha_pll_configure);
 
 static int clk_alpha_pll_hwfsm_enable(struct clk_hw *hw)
 {