Re: [PATCH 2/2] clk: qcom: Add video clock controller driver for SDM845

2018-05-02 Thread Nischal, Amit



On 5/2/2018 3:11 AM, Stephen Boyd wrote:

Quoting Amit Nischal (2018-04-24 06:32:51)

Add support for the video clock controller found on SDM845
based devices. This would allow video drivers to probe and
control their clocks.

Signed-off-by: Amit Nischal 

Driver looks small and good. Only concern is that there aren't resets
defined, but the binding says there are resets. Will you add them?

Thanks for the review.
As of now, there are no resets defined for VIDEOCC. So I will move
the #reset-cells tooptional property in the dt-binding.
Same will be updated in the next patch series.


I'm going to wait for the respin on the dts binding anyway.

Yes, I will send the next patch series soon.

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




Re: [PATCH 2/2] clk: qcom: Add video clock controller driver for SDM845

2018-05-02 Thread Nischal, Amit



On 5/2/2018 3:11 AM, Stephen Boyd wrote:

Quoting Amit Nischal (2018-04-24 06:32:51)

Add support for the video clock controller found on SDM845
based devices. This would allow video drivers to probe and
control their clocks.

Signed-off-by: Amit Nischal 

Driver looks small and good. Only concern is that there aren't resets
defined, but the binding says there are resets. Will you add them?

Thanks for the review.
As of now, there are no resets defined for VIDEOCC. So I will move
the #reset-cells tooptional property in the dt-binding.
Same will be updated in the next patch series.


I'm going to wait for the respin on the dts binding anyway.

Yes, I will send the next patch series soon.

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




Re: [PATCH v2 1/4] clk: qcom: Clear hardware clock control bit of RCG

2018-03-25 Thread Nischal, Amit



On 3/20/2018 4:25 AM, Stephen Boyd wrote:

Quoting Amit Nischal (2018-03-07 23:18:12)

For upcoming targets like sdm845, POR value of the hardware clock control
bit is set for most of root clocks which needs to be cleared for software
to be able to control. For older targets like MSM8996, this bit is reserved
bit and having POR value as 0 so this patch will work for the older targets
too. So update the configuration mask to take care of the same to clear
hardware clock control bit.

Signed-off-by: Amit Nischal 
---
  drivers/clk/qcom/clk-rcg2.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index bbeaf9c..e63db10 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013, 2018, The Linux Foundation. All rights reserved.

It would be nice if lawyers over there could avoid forcing copyright
date updates when less than half the file changes.


Thanks for the review.
I will address the above in the next patch series.




   *
   * This software is licensed under the terms of the GNU General Public
   * License version 2, as published by the Free Software Foundation, and
@@ -42,6 +42,7 @@
  #define CFG_MODE_SHIFT 12
  #define CFG_MODE_MASK  (0x3 << CFG_MODE_SHIFT)
  #define CFG_MODE_DUAL_EDGE (0x2 << CFG_MODE_SHIFT)
+#define CFG_HW_CLK_CTRL_MASK   BIT(20)

  #define M_REG  0x8
  #define N_REG  0xc
@@ -276,7 +277,7 @@ static int clk_rcg2_configure(struct clk_rcg2 *rcg, const 
struct freq_tbl *f)
 }

 mask = BIT(rcg->hid_width) - 1;
-   mask |= CFG_SRC_SEL_MASK | CFG_MODE_MASK;
+   mask |= CFG_SRC_SEL_MASK | CFG_MODE_MASK | CFG_HW_CLK_CTRL_MASK;
 cfg = f->pre_div << CFG_SRC_DIV_SHIFT;
 cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
 if (rcg->mnd_width && f->n && (f->m != f->n))

Is there going to be a future patch to update the RCGs to indicate they
support hardware control or not?


As of now, there will not be any patch to update the RCGs to support HW control.



Re: [PATCH v2 1/4] clk: qcom: Clear hardware clock control bit of RCG

2018-03-25 Thread Nischal, Amit



On 3/20/2018 4:25 AM, Stephen Boyd wrote:

Quoting Amit Nischal (2018-03-07 23:18:12)

For upcoming targets like sdm845, POR value of the hardware clock control
bit is set for most of root clocks which needs to be cleared for software
to be able to control. For older targets like MSM8996, this bit is reserved
bit and having POR value as 0 so this patch will work for the older targets
too. So update the configuration mask to take care of the same to clear
hardware clock control bit.

Signed-off-by: Amit Nischal 
---
  drivers/clk/qcom/clk-rcg2.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index bbeaf9c..e63db10 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013, 2018, The Linux Foundation. All rights reserved.

It would be nice if lawyers over there could avoid forcing copyright
date updates when less than half the file changes.


Thanks for the review.
I will address the above in the next patch series.




   *
   * This software is licensed under the terms of the GNU General Public
   * License version 2, as published by the Free Software Foundation, and
@@ -42,6 +42,7 @@
  #define CFG_MODE_SHIFT 12
  #define CFG_MODE_MASK  (0x3 << CFG_MODE_SHIFT)
  #define CFG_MODE_DUAL_EDGE (0x2 << CFG_MODE_SHIFT)
+#define CFG_HW_CLK_CTRL_MASK   BIT(20)

  #define M_REG  0x8
  #define N_REG  0xc
@@ -276,7 +277,7 @@ static int clk_rcg2_configure(struct clk_rcg2 *rcg, const 
struct freq_tbl *f)
 }

 mask = BIT(rcg->hid_width) - 1;
-   mask |= CFG_SRC_SEL_MASK | CFG_MODE_MASK;
+   mask |= CFG_SRC_SEL_MASK | CFG_MODE_MASK | CFG_HW_CLK_CTRL_MASK;
 cfg = f->pre_div << CFG_SRC_DIV_SHIFT;
 cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
 if (rcg->mnd_width && f->n && (f->m != f->n))

Is there going to be a future patch to update the RCGs to indicate they
support hardware control or not?


As of now, there will not be any patch to update the RCGs to support HW control.



Re: [PATCH v2] clk: qcom: Add Global Clock controller (GCC) driver for SDM845

2018-01-31 Thread Nischal, Amit



On 1/27/2018 5:26 AM, Stephen Boyd wrote:

On 01/22, Amit Nischal wrote:

Add support for the global clock controller found on SDM845
based devices. This should allow most non-multimedia device
drivers to probe and control their clocks.

Signed-off-by: Taniya Das 

Is Taniya the author? Should be a From: line then.

Thanks for the review. I will change the author name.

Signed-off-by: Amit Nischal 
---

This patch is dependent on below changes:
1. https://patchwork.kernel.org/patch/10139991/
2. https://patchwork.kernel.org/patch/10139987/
3. https://patchwork.kernel.org/patch/10144621/

Ok. Next time you can send them all again in a series to make it
easier for me.

Yes, I will post all the patches in a series after fixing review findings.



diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index fbf4532..91e4557 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -226,3 +226,12 @@ config SPMI_PMIC_CLKDIV
  Technologies, Inc. SPMI PMIC. It configures the frequency of
  clkdiv outputs of the PMIC. These clocks are typically wired
  through alternate functions on GPIO pins.
+
+config SDM_GCC_845
+   tristate "SDM845 Global Clock Controller"
+   depends on COMMON_CLK_QCOM
+   help
+ Support for the global clock controller on Qualcomm Technologies, Inc
+ sdm845 devices.
+ Say Y if you want to use peripheral devices such as UART, SPI,
+ i2c, USB, UFS, SD/eMMC, PCIe, etc.

Put this in sorted order in the Kconfig? Or at least try to.  I
should go back and sort the rest later.

ok sure. I will do that.



diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 230332c..1aa23b3 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
  obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
  obj-$(CONFIG_MSM_GCC_8994) += gcc-msm8994.o
  obj-$(CONFIG_MSM_GCC_8996) += gcc-msm8996.o
+obj-$(CONFIG_SDM_GCC_845)  += gcc-sdm845.o

This should be sorted too.

Done.



  obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
  obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
  obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
new file mode 100644
index 000..fe62d87
--- /dev/null
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -0,0 +1,3633 @@
+// 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 
+#include 
+
+#include 
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+#include "clk-alpha-pll.h"
+#include "gdsc.h"
+#include "reset.h"
+
+#define GCC_MMSS_MISC  0x09FFC

Please drop the onetime defines.

ok. Will fix this in the next series.



+#define GCC_GPU_MISC   0x71028
+
+#define CXO_FREQUENCY  1920
+
+#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
+
+static struct freq_tbl cxo_safe_src_f = {

const?


+   .freq = CXO_FREQUENCY,
+   .src = 0,
+   .pre_div = 1,
+   .m = 0,
+   .n = 0,
+};

All those = 0 are useless. But CXO may not always be 19.2 MHz so
that's a non-starter. Try and figure out how to get rid of this?
Thanks for this finding. Will read the safe source's frequency inside 
the probe

of each CC driver and assign the rate to the 'freq' member variable of
'cxo_safe_src_f' structure as we have 'bi_tcxo' fixed clock and its rate 
is defined

in SDM845's main device tree file.



+
+enum {
+   P_BI_TCXO,
+   P_AUD_REF_CLK,
+   P_CORE_BI_PLL_TEST_SE,
+   P_GPLL0_OUT_EVEN,
+   P_GPLL0_OUT_MAIN,
+   P_GPLL4_OUT_MAIN,
+   P_SLEEP_CLK,
+};
+
+static const struct parent_map gcc_parent_map_0[] = {
+   { P_BI_TCXO, 0 },
+   { P_GPLL0_OUT_MAIN, 1 },
+   { P_GPLL0_OUT_EVEN, 6 },
+   { P_CORE_BI_PLL_TEST_SE, 7 },

[...]

+
+static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] = {
+   F(40, P_BI_TCXO, 12, 1, 4),
+   F(960, P_BI_TCXO, 2, 0, 0),
+   F(1920, P_BI_TCXO, 1, 0, 0),
+   F(2500, P_GPLL0_OUT_EVEN, 12, 0, 0),
+   F(5000, P_GPLL0_OUT_EVEN, 6, 0, 0),
+   F(1, P_GPLL0_OUT_MAIN, 6, 0, 0),
+   F(20150, P_GPLL4_OUT_MAIN, 4, 0, 0),
+   { }
+};
+
+static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
+   .cmd_rcgr = 0x1400c,
+   .mnd_width = 8,
+   .hid_width = 5,
+   .parent_map = gcc_parent_map_10,
+   .freq_tbl = ftbl_gcc_sdcc2_apps_clk_src,
+   .safe_src_freq_tbl = _safe_src_f,
+   .clkr.hw.init = &(struct clk_init_data){
+   .name = "gcc_sdcc2_apps_clk_src",
+   .parent_names = gcc_parent_names_10,
+   .num_parents = 5,
+

Re: [PATCH v2] clk: qcom: Add Global Clock controller (GCC) driver for SDM845

2018-01-31 Thread Nischal, Amit



On 1/27/2018 5:26 AM, Stephen Boyd wrote:

On 01/22, Amit Nischal wrote:

Add support for the global clock controller found on SDM845
based devices. This should allow most non-multimedia device
drivers to probe and control their clocks.

Signed-off-by: Taniya Das 

Is Taniya the author? Should be a From: line then.

Thanks for the review. I will change the author name.

Signed-off-by: Amit Nischal 
---

This patch is dependent on below changes:
1. https://patchwork.kernel.org/patch/10139991/
2. https://patchwork.kernel.org/patch/10139987/
3. https://patchwork.kernel.org/patch/10144621/

Ok. Next time you can send them all again in a series to make it
easier for me.

Yes, I will post all the patches in a series after fixing review findings.



diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index fbf4532..91e4557 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -226,3 +226,12 @@ config SPMI_PMIC_CLKDIV
  Technologies, Inc. SPMI PMIC. It configures the frequency of
  clkdiv outputs of the PMIC. These clocks are typically wired
  through alternate functions on GPIO pins.
+
+config SDM_GCC_845
+   tristate "SDM845 Global Clock Controller"
+   depends on COMMON_CLK_QCOM
+   help
+ Support for the global clock controller on Qualcomm Technologies, Inc
+ sdm845 devices.
+ Say Y if you want to use peripheral devices such as UART, SPI,
+ i2c, USB, UFS, SD/eMMC, PCIe, etc.

Put this in sorted order in the Kconfig? Or at least try to.  I
should go back and sort the rest later.

ok sure. I will do that.



diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 230332c..1aa23b3 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
  obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
  obj-$(CONFIG_MSM_GCC_8994) += gcc-msm8994.o
  obj-$(CONFIG_MSM_GCC_8996) += gcc-msm8996.o
+obj-$(CONFIG_SDM_GCC_845)  += gcc-sdm845.o

This should be sorted too.

Done.



  obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
  obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
  obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
new file mode 100644
index 000..fe62d87
--- /dev/null
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -0,0 +1,3633 @@
+// 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 
+#include 
+
+#include 
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+#include "clk-alpha-pll.h"
+#include "gdsc.h"
+#include "reset.h"
+
+#define GCC_MMSS_MISC  0x09FFC

Please drop the onetime defines.

ok. Will fix this in the next series.



+#define GCC_GPU_MISC   0x71028
+
+#define CXO_FREQUENCY  1920
+
+#define F(f, s, h, m, n) { (f), (s), (2 * (h) - 1), (m), (n) }
+
+static struct freq_tbl cxo_safe_src_f = {

const?


+   .freq = CXO_FREQUENCY,
+   .src = 0,
+   .pre_div = 1,
+   .m = 0,
+   .n = 0,
+};

All those = 0 are useless. But CXO may not always be 19.2 MHz so
that's a non-starter. Try and figure out how to get rid of this?
Thanks for this finding. Will read the safe source's frequency inside 
the probe

of each CC driver and assign the rate to the 'freq' member variable of
'cxo_safe_src_f' structure as we have 'bi_tcxo' fixed clock and its rate 
is defined

in SDM845's main device tree file.



+
+enum {
+   P_BI_TCXO,
+   P_AUD_REF_CLK,
+   P_CORE_BI_PLL_TEST_SE,
+   P_GPLL0_OUT_EVEN,
+   P_GPLL0_OUT_MAIN,
+   P_GPLL4_OUT_MAIN,
+   P_SLEEP_CLK,
+};
+
+static const struct parent_map gcc_parent_map_0[] = {
+   { P_BI_TCXO, 0 },
+   { P_GPLL0_OUT_MAIN, 1 },
+   { P_GPLL0_OUT_EVEN, 6 },
+   { P_CORE_BI_PLL_TEST_SE, 7 },

[...]

+
+static const struct freq_tbl ftbl_gcc_sdcc2_apps_clk_src[] = {
+   F(40, P_BI_TCXO, 12, 1, 4),
+   F(960, P_BI_TCXO, 2, 0, 0),
+   F(1920, P_BI_TCXO, 1, 0, 0),
+   F(2500, P_GPLL0_OUT_EVEN, 12, 0, 0),
+   F(5000, P_GPLL0_OUT_EVEN, 6, 0, 0),
+   F(1, P_GPLL0_OUT_MAIN, 6, 0, 0),
+   F(20150, P_GPLL4_OUT_MAIN, 4, 0, 0),
+   { }
+};
+
+static struct clk_rcg2 gcc_sdcc2_apps_clk_src = {
+   .cmd_rcgr = 0x1400c,
+   .mnd_width = 8,
+   .hid_width = 5,
+   .parent_map = gcc_parent_map_10,
+   .freq_tbl = ftbl_gcc_sdcc2_apps_clk_src,
+   .safe_src_freq_tbl = _safe_src_f,
+   .clkr.hw.init = &(struct clk_init_data){
+   .name = "gcc_sdcc2_apps_clk_src",
+   .parent_names = gcc_parent_names_10,
+   .num_parents = 5,
+   .flags = CLK_SET_RATE_PARENT,
+