Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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 > > > > > > > > > > > > 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 > > > > > > Signed-off-by: Taniya Das > > > > > > Co-developed-by: Taniya Das > > > > > > Signed-off-by: Anu Ramanathan > > > > > > [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 > > > > > > Signed-off-by: Vinod Koul > > > > 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.anders...@linaro.org: Clean and tidy] > Signed-off-by: Bjorn Andersson > [vk...@kernel.org: Clean and tidy even more] > Signed-off-by: Vinod Koul > > 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] = > > > > > > _usb_hs_phy_cfg_ahb_clk.clkr, > > > > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr, > > > > > > + [GFX3D_CLK_SRC] = _clk_src.clkr, > > > > > > + [GP1_CLK_SRC] = _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
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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 > > > > > > > > > > > > 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 > > > > > > Signed-off-by: Taniya Das > > > > > > Co-developed-by: Taniya Das > > > > > > Signed-off-by: Anu Ramanathan > > > > > > [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 > > > > > > Signed-off-by: Vinod Koul > > > > 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.anders...@linaro.org: Clean and tidy] > Signed-off-by: Bjorn Andersson > [vk...@kernel.org: Clean and tidy even more] > Signed-off-by: Vinod Koul > > 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] = > > > > > > _usb_hs_phy_cfg_ahb_clk.clkr, > > > > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr, > > > > > > + [GFX3D_CLK_SRC] = _clk_src.clkr, > > > > > > + [GP1_CLK_SRC] = _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
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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 > > > > > > > > > > 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 > > > > > Signed-off-by: Taniya Das > > > > > Co-developed-by: Taniya Das > > > > > Signed-off-by: Anu Ramanathan > > > > > [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 > > > > > Signed-off-by: Vinod Koul > > 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.anders...@linaro.org: Clean and tidy] Signed-off-by: Bjorn Andersson [vk...@kernel.org: Clean and tidy even more] Signed-off-by: Vinod Koul Would be the kernel standard for maintainer tags. > > > > > > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = > > > > > _usb_hs_phy_cfg_ahb_clk.clkr, > > > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr, > > > > > + [GFX3D_CLK_SRC] = _clk_src.clkr, > > > > > + [GP1_CLK_SRC] = _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.
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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 > > > > > > > > > > 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 > > > > > Signed-off-by: Taniya Das > > > > > Co-developed-by: Taniya Das > > > > > Signed-off-by: Anu Ramanathan > > > > > [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 > > > > > Signed-off-by: Vinod Koul > > 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.anders...@linaro.org: Clean and tidy] Signed-off-by: Bjorn Andersson [vk...@kernel.org: Clean and tidy even more] Signed-off-by: Vinod Koul Would be the kernel standard for maintainer tags. > > > > > > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = > > > > > _usb_hs_phy_cfg_ahb_clk.clkr, > > > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr, > > > > > + [GFX3D_CLK_SRC] = _clk_src.clkr, > > > > > + [GP1_CLK_SRC] = _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.
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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_SRC1
+#define BLSP1_QUP0_SPI_APPS_CLK_SRC2
+#define BLSP1_QUP1_I2C_APPS_CLK_SRC3
+#define BLSP1_QUP1_SPI_APPS_CLK_SRC4
+#define BLSP1_QUP2_I2C_APPS_CLK_SRC5
+#define BLSP1_QUP2_SPI_APPS_CLK_SRC6
+#define BLSP1_QUP3_I2C_APPS_CLK_SRC7
+#define BLSP1_QUP3_SPI_APPS_CLK_SRC8
+#define BLSP1_QUP4_I2C_APPS_CLK_SRC9
+#define BLSP1_QUP4_SPI_APPS_CLK_SRC10
+#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_SRC15
+#define BLSP2_QUP0_SPI_APPS_CLK_SRC16
+#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_CLK28
+#define GCC_BLSP1_QUP0_SPI_APPS_CLK29
+#define GCC_BLSP1_QUP1_I2C_APPS_CLK30
+#define GCC_BLSP1_QUP1_SPI_APPS_CLK31
+#define GCC_BLSP1_QUP2_I2C_APPS_CLK32
+#define GCC_BLSP1_QUP2_SPI_APPS_CLK33
+#define GCC_BLSP1_QUP3_I2C_APPS_CLK34
+#define GCC_BLSP1_QUP3_SPI_APPS_CLK35
+#define GCC_BLSP1_QUP4_I2C_APPS_CLK36
+#define GCC_BLSP1_QUP4_SPI_APPS_CLK37
+#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_CLK43
+#define GCC_BLSP2_QUP0_SPI_APPS_CLK44
+#define GCC_BLSP2_UART0_APPS_CLK 45
+#define GCC_BOOT_ROM_AHB_CLK 46
+#define GCC_DCC_CLK47
+#define GCC_GENI_IR_H_CLK 48
+#define GCC_ETH_AXI_CLK49
+#define GCC_ETH_PTP_CLK50
+#define GCC_ETH_RGMII_CLK 51
+#define GCC_ETH_SLAVE_AHB_CLK 52
+#define GCC_GENI_IR_S_CLK 53
+#define GCC_GP1_CLK54
+#define GCC_GP2_CLK55
+#define GCC_GP3_CLK56
+#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
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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_SRC1
+#define BLSP1_QUP0_SPI_APPS_CLK_SRC2
+#define BLSP1_QUP1_I2C_APPS_CLK_SRC3
+#define BLSP1_QUP1_SPI_APPS_CLK_SRC4
+#define BLSP1_QUP2_I2C_APPS_CLK_SRC5
+#define BLSP1_QUP2_SPI_APPS_CLK_SRC6
+#define BLSP1_QUP3_I2C_APPS_CLK_SRC7
+#define BLSP1_QUP3_SPI_APPS_CLK_SRC8
+#define BLSP1_QUP4_I2C_APPS_CLK_SRC9
+#define BLSP1_QUP4_SPI_APPS_CLK_SRC10
+#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_SRC15
+#define BLSP2_QUP0_SPI_APPS_CLK_SRC16
+#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_CLK28
+#define GCC_BLSP1_QUP0_SPI_APPS_CLK29
+#define GCC_BLSP1_QUP1_I2C_APPS_CLK30
+#define GCC_BLSP1_QUP1_SPI_APPS_CLK31
+#define GCC_BLSP1_QUP2_I2C_APPS_CLK32
+#define GCC_BLSP1_QUP2_SPI_APPS_CLK33
+#define GCC_BLSP1_QUP3_I2C_APPS_CLK34
+#define GCC_BLSP1_QUP3_SPI_APPS_CLK35
+#define GCC_BLSP1_QUP4_I2C_APPS_CLK36
+#define GCC_BLSP1_QUP4_SPI_APPS_CLK37
+#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_CLK43
+#define GCC_BLSP2_QUP0_SPI_APPS_CLK44
+#define GCC_BLSP2_UART0_APPS_CLK 45
+#define GCC_BOOT_ROM_AHB_CLK 46
+#define GCC_DCC_CLK47
+#define GCC_GENI_IR_H_CLK 48
+#define GCC_ETH_AXI_CLK49
+#define GCC_ETH_PTP_CLK50
+#define GCC_ETH_RGMII_CLK 51
+#define GCC_ETH_SLAVE_AHB_CLK 52
+#define GCC_GENI_IR_S_CLK 53
+#define GCC_GP1_CLK54
+#define GCC_GP2_CLK55
+#define GCC_GP3_CLK56
+#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
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
> > > >
> > > > 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
> > > > Signed-off-by: Taniya Das
> > > > Co-developed-by: Taniya Das
> > > > Signed-off-by: Anu Ramanathan
> > > > [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
> > > > Signed-off-by: Vinod Koul
Sorry not sure I understand, Bjorn and me did cleanup and we signed-off
per process, did I miss something?
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > >
> > > 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 = _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] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > > > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > > > + [GP1_CLK_SRC] = _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
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
> > > >
> > > > 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
> > > > Signed-off-by: Taniya Das
> > > > Co-developed-by: Taniya Das
> > > > Signed-off-by: Anu Ramanathan
> > > > [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
> > > > Signed-off-by: Vinod Koul
Sorry not sure I understand, Bjorn and me did cleanup and we signed-off
per process, did I miss something?
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > >
> > > 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 = _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] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > > > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > > > + [GP1_CLK_SRC] = _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
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
> > >
> > > 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
> > > Signed-off-by: Taniya Das
> > > Co-developed-by: Taniya Das
> > > Signed-off-by: Anu Ramanathan
> > > [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
> > > Signed-off-by: Vinod Koul
> > > ---
> > > - 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
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> >
> > 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.
>
> > > +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 = _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.
>
> > > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > > + [GP1_CLK_SRC] = _clk_src.clkr,
> >
> > Why are some of these missing GCC_ prefix?
>
> will add..
Thanks!
>
> > > +static int gcc_qcs404_probe(struct platform_device *pdev)
> > > +{
> > > + struct regmap *regmap;
> > > + int ret;
> > > +
> > > + ret = qcom_cc_register_board_clk(>dev,
> > > +"xo_board", "cxo", 1920);
> >
> > 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
Yes it would be similar to 8916 or 8996/845.
>
> >
> > > + if (ret)
> > > + return ret;
> > > +
> > > + regmap = qcom_cc_map(pdev, _qcs404_desc);
> > > + if (IS_ERR(regmap))
> > > + return PTR_ERR(regmap);
> > > +
> > > + clk_alpha_pll_configure(_out_main, regmap, _config);
> > > + clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 1920);
> >
> > 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?
You mark the clk flags with CLK_IS_CRITICAL.
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
> > >
> > > 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
> > > Signed-off-by: Taniya Das
> > > Co-developed-by: Taniya Das
> > > Signed-off-by: Anu Ramanathan
> > > [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
> > > Signed-off-by: Vinod Koul
> > > ---
> > > - 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
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> >
> > 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.
>
> > > +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 = _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.
>
> > > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > > + [GP1_CLK_SRC] = _clk_src.clkr,
> >
> > Why are some of these missing GCC_ prefix?
>
> will add..
Thanks!
>
> > > +static int gcc_qcs404_probe(struct platform_device *pdev)
> > > +{
> > > + struct regmap *regmap;
> > > + int ret;
> > > +
> > > + ret = qcom_cc_register_board_clk(>dev,
> > > +"xo_board", "cxo", 1920);
> >
> > 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
Yes it would be similar to 8916 or 8996/845.
>
> >
> > > + if (ret)
> > > + return ret;
> > > +
> > > + regmap = qcom_cc_map(pdev, _qcs404_desc);
> > > + if (IS_ERR(regmap))
> > > + return PTR_ERR(regmap);
> > > +
> > > + clk_alpha_pll_configure(_out_main, regmap, _config);
> > > + clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 1920);
> >
> > 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?
You mark the clk flags with CLK_IS_CRITICAL.
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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_SRC1
> > +#define BLSP1_QUP0_SPI_APPS_CLK_SRC2
> > +#define BLSP1_QUP1_I2C_APPS_CLK_SRC3
> > +#define BLSP1_QUP1_SPI_APPS_CLK_SRC4
> > +#define BLSP1_QUP2_I2C_APPS_CLK_SRC5
> > +#define BLSP1_QUP2_SPI_APPS_CLK_SRC6
> > +#define BLSP1_QUP3_I2C_APPS_CLK_SRC7
> > +#define BLSP1_QUP3_SPI_APPS_CLK_SRC8
> > +#define BLSP1_QUP4_I2C_APPS_CLK_SRC9
> > +#define BLSP1_QUP4_SPI_APPS_CLK_SRC10
> > +#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_SRC15
> > +#define BLSP2_QUP0_SPI_APPS_CLK_SRC16
> > +#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_CLK28
> > +#define GCC_BLSP1_QUP0_SPI_APPS_CLK29
> > +#define GCC_BLSP1_QUP1_I2C_APPS_CLK30
> > +#define GCC_BLSP1_QUP1_SPI_APPS_CLK31
> > +#define GCC_BLSP1_QUP2_I2C_APPS_CLK32
> > +#define GCC_BLSP1_QUP2_SPI_APPS_CLK33
> > +#define GCC_BLSP1_QUP3_I2C_APPS_CLK34
> > +#define GCC_BLSP1_QUP3_SPI_APPS_CLK35
> > +#define GCC_BLSP1_QUP4_I2C_APPS_CLK36
> > +#define GCC_BLSP1_QUP4_SPI_APPS_CLK37
> > +#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_CLK43
> > +#define GCC_BLSP2_QUP0_SPI_APPS_CLK44
> > +#define GCC_BLSP2_UART0_APPS_CLK 45
> > +#define GCC_BOOT_ROM_AHB_CLK 46
> > +#define GCC_DCC_CLK47
> > +#define GCC_GENI_IR_H_CLK 48
> > +#define GCC_ETH_AXI_CLK49
> > +#define GCC_ETH_PTP_CLK50
> > +#define GCC_ETH_RGMII_CLK 51
> > +#define GCC_ETH_SLAVE_AHB_CLK 52
> > +#define GCC_GENI_IR_S_CLK 53
> > +#define GCC_GP1_CLK54
> > +#define GCC_GP2_CLK55
> > +#define GCC_GP3_CLK56
> > +#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
> >
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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_SRC1
> > +#define BLSP1_QUP0_SPI_APPS_CLK_SRC2
> > +#define BLSP1_QUP1_I2C_APPS_CLK_SRC3
> > +#define BLSP1_QUP1_SPI_APPS_CLK_SRC4
> > +#define BLSP1_QUP2_I2C_APPS_CLK_SRC5
> > +#define BLSP1_QUP2_SPI_APPS_CLK_SRC6
> > +#define BLSP1_QUP3_I2C_APPS_CLK_SRC7
> > +#define BLSP1_QUP3_SPI_APPS_CLK_SRC8
> > +#define BLSP1_QUP4_I2C_APPS_CLK_SRC9
> > +#define BLSP1_QUP4_SPI_APPS_CLK_SRC10
> > +#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_SRC15
> > +#define BLSP2_QUP0_SPI_APPS_CLK_SRC16
> > +#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_CLK28
> > +#define GCC_BLSP1_QUP0_SPI_APPS_CLK29
> > +#define GCC_BLSP1_QUP1_I2C_APPS_CLK30
> > +#define GCC_BLSP1_QUP1_SPI_APPS_CLK31
> > +#define GCC_BLSP1_QUP2_I2C_APPS_CLK32
> > +#define GCC_BLSP1_QUP2_SPI_APPS_CLK33
> > +#define GCC_BLSP1_QUP3_I2C_APPS_CLK34
> > +#define GCC_BLSP1_QUP3_SPI_APPS_CLK35
> > +#define GCC_BLSP1_QUP4_I2C_APPS_CLK36
> > +#define GCC_BLSP1_QUP4_SPI_APPS_CLK37
> > +#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_CLK43
> > +#define GCC_BLSP2_QUP0_SPI_APPS_CLK44
> > +#define GCC_BLSP2_UART0_APPS_CLK 45
> > +#define GCC_BOOT_ROM_AHB_CLK 46
> > +#define GCC_DCC_CLK47
> > +#define GCC_GENI_IR_H_CLK 48
> > +#define GCC_ETH_AXI_CLK49
> > +#define GCC_ETH_PTP_CLK50
> > +#define GCC_ETH_RGMII_CLK 51
> > +#define GCC_ETH_SLAVE_AHB_CLK 52
> > +#define GCC_GENI_IR_S_CLK 53
> > +#define GCC_GP1_CLK54
> > +#define GCC_GP2_CLK55
> > +#define GCC_GP3_CLK56
> > +#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
> >
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
Hi Tanya,
On 06-10-18, 23:19, Taniya Das wrote:
> > > > +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 = _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.
Any specific reason for that?
>
> > > > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > > > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > > > + [GP1_CLK_SRC] = _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.
That's right but I think Stephan wants this namespaced properly which IMO
makes sense. Btw looking at other examples I saw that drivers are using
GCC_ tag even if HW representation does not have that
> > > > +static int gcc_qcs404_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct regmap *regmap;
> > > > + int ret;
> > > > +
> > > > + ret = qcom_cc_register_board_clk(>dev,
> > > > +"xo_board", "cxo", 1920);
> > >
> > > 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.
OK will check this
> > > > +#define GCC_GENI_IR_BCR0
> > > > +#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_BCR8
> > > > +#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.
Okay will check and add
--
~Vinod
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
Hi Tanya,
On 06-10-18, 23:19, Taniya Das wrote:
> > > > +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 = _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.
Any specific reason for that?
>
> > > > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > > > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > > > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > > > + [GP1_CLK_SRC] = _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.
That's right but I think Stephan wants this namespaced properly which IMO
makes sense. Btw looking at other examples I saw that drivers are using
GCC_ tag even if HW representation does not have that
> > > > +static int gcc_qcs404_probe(struct platform_device *pdev)
> > > > +{
> > > > + struct regmap *regmap;
> > > > + int ret;
> > > > +
> > > > + ret = qcom_cc_register_board_clk(>dev,
> > > > +"xo_board", "cxo", 1920);
> > >
> > > 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.
OK will check this
> > > > +#define GCC_GENI_IR_BCR0
> > > > +#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_BCR8
> > > > +#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.
Okay will check and add
--
~Vinod
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
On 9/22/2018 12:29 AM, Vinod Koul wrote:
From: Shefali Jain
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
Signed-off-by: Taniya Das
Co-developed-by: Taniya Das
Signed-off-by: Anu Ramanathan
[rebase and tidyup for upstream]
Signed-off-by: Bjorn Andersson
Signed-off-by: Vinod Koul
---
.../devicetree/bindings/clock/qcom,gcc.txt |1 +
drivers/clk/qcom/Kconfig |8 +
drivers/clk/qcom/Makefile |1 +
drivers/clk/qcom/gcc-qcs404.c | 2729
include/dt-bindings/clock/qcom,gcc-qcs404.h| 166 ++
5 files changed, 2905 insertions(+)
create mode 100644 drivers/clk/qcom/gcc-qcs404.c
create mode 100644 include/dt-bindings/clock/qcom,gcc-qcs404.h
diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.txt
b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
index 664ea1fd6c76..69fa8603b5ab 100644
--- a/Documentation/devicetree/bindings/clock/qcom,gcc.txt
+++ b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
@@ -19,6 +19,7 @@ Required properties :
"qcom,gcc-msm8996"
"qcom,gcc-msm8998"
"qcom,gcc-mdm9615"
+ "qcom,gcc-qcs404"
"qcom,gcc-sdm845"
- 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.
+
config SDM_GCC_845
tristate "SDM845 Global Clock Controller"
select QCOM_GDSC
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 21a45035930d..37197b90ddf5 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
obj-$(CONFIG_QCOM_CLK_RPMH) += clk-rpmh.o
obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
obj-$(CONFIG_SDM_DISPCC_845) += dispcc-sdm845.o
+obj-$(CONFIG_QCS_GCC_404) += gcc-qcs404.o
obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o
obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
new file mode 100644
index ..6d1387ef798b
--- /dev/null
+++ b/drivers/clk/qcom/gcc-qcs404.c
@@ -0,0 +1,2729 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "common.h"
+#include "reset.h"
+
+enum {
+ P_CORE_BI_PLL_TEST_SE,
+ P_DSI0_PHY_PLL_OUT_BYTECLK,
+ P_DSI0_PHY_PLL_OUT_DSICLK,
+ P_GPLL0_OUT_AUX,
+ P_GPLL0_OUT_MAIN,
+ P_GPLL1_OUT_MAIN,
+ P_GPLL3_OUT_MAIN,
+ P_GPLL4_OUT_AUX,
+ P_GPLL4_OUT_MAIN,
+ P_GPLL6_OUT_AUX,
+ P_HDMI_PHY_PLL_CLK,
+ P_PCIE_0_PIPE_CLK,
+ P_SLEEP_CLK,
+ P_XO,
+};
+
+static const struct parent_map gcc_parent_map_0[] = {
+ { P_XO, 0 },
+ { P_GPLL0_OUT_MAIN, 1 },
+ { P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const gcc_parent_names_0[] = {
+ "cxo",
+ "gpll0_out_main",
+ "core_bi_pll_test_se",
+};
+
+static const char * const gcc_parent_names_ao_0[] = {
+ "cxo_a",
+ "gpll0_ao_out_main",
+ "core_bi_pll_test_se",
+};
+
+static const struct parent_map gcc_parent_map_1[] = {
+ { P_XO, 0 },
+ { P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const gcc_parent_names_1[] = {
+ "cxo",
+ "core_bi_pll_test_se",
+};
+
+static const struct parent_map gcc_parent_map_2[] = {
+ { P_XO, 0 },
+ { P_GPLL0_OUT_MAIN, 1 },
+ { P_GPLL6_OUT_AUX, 2 },
+ { P_SLEEP_CLK, 6 },
+};
+
+static const char * const gcc_parent_names_2[] = {
+ "cxo",
+ "gpll0_out_main",
+ "gpll6_out_aux",
+ "sleep_clk",
+};
+
+static const struct parent_map gcc_parent_map_3[] = {
+ { P_XO, 0 },
+ { P_GPLL0_OUT_MAIN, 1 },
+ { P_GPLL6_OUT_AUX, 2 },
+ { P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char *
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
On 9/22/2018 12:29 AM, Vinod Koul wrote:
From: Shefali Jain
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
Signed-off-by: Taniya Das
Co-developed-by: Taniya Das
Signed-off-by: Anu Ramanathan
[rebase and tidyup for upstream]
Signed-off-by: Bjorn Andersson
Signed-off-by: Vinod Koul
---
.../devicetree/bindings/clock/qcom,gcc.txt |1 +
drivers/clk/qcom/Kconfig |8 +
drivers/clk/qcom/Makefile |1 +
drivers/clk/qcom/gcc-qcs404.c | 2729
include/dt-bindings/clock/qcom,gcc-qcs404.h| 166 ++
5 files changed, 2905 insertions(+)
create mode 100644 drivers/clk/qcom/gcc-qcs404.c
create mode 100644 include/dt-bindings/clock/qcom,gcc-qcs404.h
diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.txt
b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
index 664ea1fd6c76..69fa8603b5ab 100644
--- a/Documentation/devicetree/bindings/clock/qcom,gcc.txt
+++ b/Documentation/devicetree/bindings/clock/qcom,gcc.txt
@@ -19,6 +19,7 @@ Required properties :
"qcom,gcc-msm8996"
"qcom,gcc-msm8998"
"qcom,gcc-mdm9615"
+ "qcom,gcc-qcs404"
"qcom,gcc-sdm845"
- 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.
+
config SDM_GCC_845
tristate "SDM845 Global Clock Controller"
select QCOM_GDSC
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 21a45035930d..37197b90ddf5 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_QCOM_CLK_RPM) += clk-rpm.o
obj-$(CONFIG_QCOM_CLK_RPMH) += clk-rpmh.o
obj-$(CONFIG_QCOM_CLK_SMD_RPM) += clk-smd-rpm.o
obj-$(CONFIG_SDM_DISPCC_845) += dispcc-sdm845.o
+obj-$(CONFIG_QCS_GCC_404) += gcc-qcs404.o
obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o
obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o
obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
new file mode 100644
index ..6d1387ef798b
--- /dev/null
+++ b/drivers/clk/qcom/gcc-qcs404.c
@@ -0,0 +1,2729 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, The Linux Foundation. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "common.h"
+#include "reset.h"
+
+enum {
+ P_CORE_BI_PLL_TEST_SE,
+ P_DSI0_PHY_PLL_OUT_BYTECLK,
+ P_DSI0_PHY_PLL_OUT_DSICLK,
+ P_GPLL0_OUT_AUX,
+ P_GPLL0_OUT_MAIN,
+ P_GPLL1_OUT_MAIN,
+ P_GPLL3_OUT_MAIN,
+ P_GPLL4_OUT_AUX,
+ P_GPLL4_OUT_MAIN,
+ P_GPLL6_OUT_AUX,
+ P_HDMI_PHY_PLL_CLK,
+ P_PCIE_0_PIPE_CLK,
+ P_SLEEP_CLK,
+ P_XO,
+};
+
+static const struct parent_map gcc_parent_map_0[] = {
+ { P_XO, 0 },
+ { P_GPLL0_OUT_MAIN, 1 },
+ { P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const gcc_parent_names_0[] = {
+ "cxo",
+ "gpll0_out_main",
+ "core_bi_pll_test_se",
+};
+
+static const char * const gcc_parent_names_ao_0[] = {
+ "cxo_a",
+ "gpll0_ao_out_main",
+ "core_bi_pll_test_se",
+};
+
+static const struct parent_map gcc_parent_map_1[] = {
+ { P_XO, 0 },
+ { P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char * const gcc_parent_names_1[] = {
+ "cxo",
+ "core_bi_pll_test_se",
+};
+
+static const struct parent_map gcc_parent_map_2[] = {
+ { P_XO, 0 },
+ { P_GPLL0_OUT_MAIN, 1 },
+ { P_GPLL6_OUT_AUX, 2 },
+ { P_SLEEP_CLK, 6 },
+};
+
+static const char * const gcc_parent_names_2[] = {
+ "cxo",
+ "gpll0_out_main",
+ "gpll6_out_aux",
+ "sleep_clk",
+};
+
+static const struct parent_map gcc_parent_map_3[] = {
+ { P_XO, 0 },
+ { P_GPLL0_OUT_MAIN, 1 },
+ { P_GPLL6_OUT_AUX, 2 },
+ { P_CORE_BI_PLL_TEST_SE, 7 },
+};
+
+static const char *
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
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
Signed-off-by: Taniya Das
Co-developed-by: Taniya Das
Signed-off-by: Anu Ramanathan
[rebase and tidyup for upstream]
Who did the tidying?
both of us :)
Signed-off-by: Bjorn Andersson
Signed-off-by: Vinod Koul
---
- 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
+#include
+#include
+#include
+#include
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 = _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] = _usb_hs_phy_cfg_ahb_clk.clkr,
+ [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
+ [GFX3D_CLK_SRC] = _clk_src.clkr,
+ [GP1_CLK_SRC] = _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(>dev,
+"xo_board", "cxo", 1920);
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, _qcs404_desc);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ clk_alpha_pll_configure(_out_main, regmap, _config);
+ clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 1920);
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_CLK89
+#define GCC_USB30_MASTER_CLK 90
+#define GCC_USB30_MOCK_UTMI_CLK91
+#define gcc_usb30_sleep_clk92
+#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_src98
+#define gp2_clk_src99
+#define gp3_clk_src100
+#define gpll0_out_main 101
+#define
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
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
Signed-off-by: Taniya Das
Co-developed-by: Taniya Das
Signed-off-by: Anu Ramanathan
[rebase and tidyup for upstream]
Who did the tidying?
both of us :)
Signed-off-by: Bjorn Andersson
Signed-off-by: Vinod Koul
---
- 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
+#include
+#include
+#include
+#include
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 = _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] = _usb_hs_phy_cfg_ahb_clk.clkr,
+ [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
+ [GFX3D_CLK_SRC] = _clk_src.clkr,
+ [GP1_CLK_SRC] = _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(>dev,
+"xo_board", "cxo", 1920);
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, _qcs404_desc);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ clk_alpha_pll_configure(_out_main, regmap, _config);
+ clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 1920);
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_CLK89
+#define GCC_USB30_MASTER_CLK 90
+#define GCC_USB30_MOCK_UTMI_CLK91
+#define gcc_usb30_sleep_clk92
+#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_src98
+#define gp2_clk_src99
+#define gp3_clk_src100
+#define gpll0_out_main 101
+#define
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
> >
> > 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
> > Signed-off-by: Taniya Das
> > Co-developed-by: Taniya Das
> > Signed-off-by: Anu Ramanathan
> > [rebase and tidyup for upstream]
>
> Who did the tidying?
both of us :)
> > Signed-off-by: Bjorn Andersson
> > Signed-off-by: Vinod Koul
> > ---
> > - 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
> > +#include
> > +#include
> > +#include
> > +#include
>
> 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 = _branch2_ops,
>
> Do these pwm clks have a parent clk of the XO?
Yes they do
> > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > + [GP1_CLK_SRC] = _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(>dev,
> > +"xo_board", "cxo", 1920);
>
> 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, _qcs404_desc);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + clk_alpha_pll_configure(_out_main, regmap, _config);
> > + clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 1920);
>
> 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_CLK89
> > +#define GCC_USB30_MASTER_CLK 90
> > +#define GCC_USB30_MOCK_UTMI_CLK91
> > +#define gcc_usb30_sleep_clk92
> > +#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_src98
> > +#define gp2_clk_src99
> > +#define gp3_clk_src
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
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
> >
> > 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
> > Signed-off-by: Taniya Das
> > Co-developed-by: Taniya Das
> > Signed-off-by: Anu Ramanathan
> > [rebase and tidyup for upstream]
>
> Who did the tidying?
both of us :)
> > Signed-off-by: Bjorn Andersson
> > Signed-off-by: Vinod Koul
> > ---
> > - 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
> > +#include
> > +#include
> > +#include
> > +#include
>
> 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 = _branch2_ops,
>
> Do these pwm clks have a parent clk of the XO?
Yes they do
> > + [GCC_USB_HS_PHY_CFG_AHB_CLK] = _usb_hs_phy_cfg_ahb_clk.clkr,
> > + [GCC_USB_HS_SYSTEM_CLK] = _usb_hs_system_clk.clkr,
> > + [GFX3D_CLK_SRC] = _clk_src.clkr,
> > + [GP1_CLK_SRC] = _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(>dev,
> > +"xo_board", "cxo", 1920);
>
> 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, _qcs404_desc);
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + clk_alpha_pll_configure(_out_main, regmap, _config);
> > + clk_set_rate(apss_ahb_clk_src.clkr.hw.clk, 1920);
>
> 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_CLK89
> > +#define GCC_USB30_MASTER_CLK 90
> > +#define GCC_USB30_MOCK_UTMI_CLK91
> > +#define gcc_usb30_sleep_clk92
> > +#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_src98
> > +#define gp2_clk_src99
> > +#define gp3_clk_src
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
Quoting Vinod Koul (2018-09-21 11:59:36)
> From: Shefali Jain
>
> 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
> Signed-off-by: Taniya Das
> Co-developed-by: Taniya Das
> Signed-off-by: Anu Ramanathan
> [rebase and tidyup for upstream]
Who did the tidying?
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Vinod Koul
> ---
> - 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.
> +
> config SDM_GCC_845
> tristate "SDM845 Global Clock Controller"
> select QCOM_GDSC
> diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
> new file mode 100644
> index ..6d1387ef798b
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-qcs404.c
> @@ -0,0 +1,2729 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
Please don't include this.
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-pll.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "common.h"
> +#include "reset.h"
[...]
> +
> +/* 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?
> + { 7, 14, 0 },
> +};
> +
> +static struct clk_alpha_pll gpll3_out_main = {
> + .offset = 0x22000,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> + .vco_table = gpll3_vco,
> + .num_vco = ARRAY_SIZE(gpll3_vco),
> + .clkr = {
> + .hw.init = &(struct clk_init_data){
> + .name = "gpll3_out_main",
> + .parent_names = (const char *[]){ "cxo" },
> + .num_parents = 1,
> + .ops = _alpha_pll_ops,
> + },
> + },
> +};
> +
[...]
> +
> +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 = _branch2_ops,
Do these pwm clks have a parent clk of the XO?
> + },
> + },
> +};
> +
> +static struct clk_branch gcc_pwm2_xo512_clk = {
> + .halt_reg = 0x4a004,
> + .halt_check = BRANCH_HALT,
> + .clkr = {
> + .enable_reg = 0x4a004,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_pwm2_xo512_clk",
> + .ops = _branch2_ops,
> + },
> + },
> +};
> +
> +static struct clk_branch gcc_qdss_dap_clk = {
[...]
> +
> +static struct clk_regmap *gcc_qcs404_clocks[] = {
> + [APSS_AHB_CLK_SRC] = _ahb_clk_src.clkr,
> + [BLSP1_QUP0_I2C_APPS_CLK_SRC] = _qup0_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP0_SPI_APPS_CLK_SRC] = _qup0_spi_apps_clk_src.clkr,
> + [BLSP1_QUP1_I2C_APPS_CLK_SRC] = _qup1_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP1_SPI_APPS_CLK_SRC] = _qup1_spi_apps_clk_src.clkr,
> + [BLSP1_QUP2_I2C_APPS_CLK_SRC] = _qup2_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP2_SPI_APPS_CLK_SRC] = _qup2_spi_apps_clk_src.clkr,
> + [BLSP1_QUP3_I2C_APPS_CLK_SRC] = _qup3_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP3_SPI_APPS_CLK_SRC] = _qup3_spi_apps_clk_src.clkr,
> + [BLSP1_QUP4_I2C_APPS_CLK_SRC] = _qup4_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP4_SPI_APPS_CLK_SRC] = _qup4_spi_apps_clk_src.clkr,
> + [BLSP1_UART0_APPS_CLK_SRC] = _uart0_apps_clk_src.clkr,
> +
Re: [PATCH 2/2] clk: qcom: gcc: Add global clock controller driver for QCS404
Quoting Vinod Koul (2018-09-21 11:59:36)
> From: Shefali Jain
>
> 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
> Signed-off-by: Taniya Das
> Co-developed-by: Taniya Das
> Signed-off-by: Anu Ramanathan
> [rebase and tidyup for upstream]
Who did the tidying?
> Signed-off-by: Bjorn Andersson
> Signed-off-by: Vinod Koul
> ---
> - 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.
> +
> config SDM_GCC_845
> tristate "SDM845 Global Clock Controller"
> select QCOM_GDSC
> diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c
> new file mode 100644
> index ..6d1387ef798b
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-qcs404.c
> @@ -0,0 +1,2729 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
Please don't include this.
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-pll.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "common.h"
> +#include "reset.h"
[...]
> +
> +/* 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?
> + { 7, 14, 0 },
> +};
> +
> +static struct clk_alpha_pll gpll3_out_main = {
> + .offset = 0x22000,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
> + .vco_table = gpll3_vco,
> + .num_vco = ARRAY_SIZE(gpll3_vco),
> + .clkr = {
> + .hw.init = &(struct clk_init_data){
> + .name = "gpll3_out_main",
> + .parent_names = (const char *[]){ "cxo" },
> + .num_parents = 1,
> + .ops = _alpha_pll_ops,
> + },
> + },
> +};
> +
[...]
> +
> +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 = _branch2_ops,
Do these pwm clks have a parent clk of the XO?
> + },
> + },
> +};
> +
> +static struct clk_branch gcc_pwm2_xo512_clk = {
> + .halt_reg = 0x4a004,
> + .halt_check = BRANCH_HALT,
> + .clkr = {
> + .enable_reg = 0x4a004,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_pwm2_xo512_clk",
> + .ops = _branch2_ops,
> + },
> + },
> +};
> +
> +static struct clk_branch gcc_qdss_dap_clk = {
[...]
> +
> +static struct clk_regmap *gcc_qcs404_clocks[] = {
> + [APSS_AHB_CLK_SRC] = _ahb_clk_src.clkr,
> + [BLSP1_QUP0_I2C_APPS_CLK_SRC] = _qup0_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP0_SPI_APPS_CLK_SRC] = _qup0_spi_apps_clk_src.clkr,
> + [BLSP1_QUP1_I2C_APPS_CLK_SRC] = _qup1_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP1_SPI_APPS_CLK_SRC] = _qup1_spi_apps_clk_src.clkr,
> + [BLSP1_QUP2_I2C_APPS_CLK_SRC] = _qup2_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP2_SPI_APPS_CLK_SRC] = _qup2_spi_apps_clk_src.clkr,
> + [BLSP1_QUP3_I2C_APPS_CLK_SRC] = _qup3_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP3_SPI_APPS_CLK_SRC] = _qup3_spi_apps_clk_src.clkr,
> + [BLSP1_QUP4_I2C_APPS_CLK_SRC] = _qup4_i2c_apps_clk_src.clkr,
> + [BLSP1_QUP4_SPI_APPS_CLK_SRC] = _qup4_spi_apps_clk_src.clkr,
> + [BLSP1_UART0_APPS_CLK_SRC] = _uart0_apps_clk_src.clkr,
> +
