[PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Linus Walleij]

These compatible strings need to be added to extend support
for the RPM CC to cover MSM8660/APQ8060. We also need to add
enumberators to the include file for a few clocks that were
missing.

Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Collect Rob's ACK
---
 Documentation/devicetree/bindings/clock/qcom,rpmcc.txt | 2 ++
 include/dt-bindings/clock/qcom,rpmcc.h                 | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
index a7235e9e1c97..d470a0187035 100644
--- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
+++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
@@ -10,6 +10,8 @@ Required properties :
 - compatible : shall contain only one of the following. The generic
                compatible "qcom,rpmcc" should be also included.
 
+			"qcom,rpmcc-msm8660", "qcom,rpmcc"
+			"qcom,rpmcc-apq8060", "qcom,rpmcc"
 			"qcom,rpmcc-msm8916", "qcom,rpmcc"
 			"qcom,rpmcc-msm8974", "qcom,rpmcc"
 			"qcom,rpmcc-apq8064", "qcom,rpmcc"
diff --git a/include/dt-bindings/clock/qcom,rpmcc.h b/include/dt-bindings/clock/qcom,rpmcc.h
index 96b63c00249e..44358562a031 100644
--- a/include/dt-bindings/clock/qcom,rpmcc.h
+++ b/include/dt-bindings/clock/qcom,rpmcc.h
@@ -37,6 +37,10 @@
 #define RPM_SYS_FABRIC_A_CLK			19
 #define RPM_SFPB_CLK				20
 #define RPM_SFPB_A_CLK				21
+#define RPM_PLL4_CLK				22
+#define RPM_PLL4_A_CLK				23
+#define RPM_SMI_CLK				24
+#define RPM_SMI_A_CLK				25
 
 /* SMD RPM clocks */
 #define RPM_SMD_XO_CLK_SRC				0
-- 
2.9.3

[PATCH 2/5 v2] clk: qcom: Elaborate on "active" clocks in the RPM clock bindings

[Linus Walleij]

The concept of "active" clocks is just explained in a bried comment in the
device driver, let's explain it a bit more in the device tree bindings
so everyone understands this.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Reword slighty in accordance with Stephens feedback.
---
 Documentation/devicetree/bindings/clock/qcom,rpmcc.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
index d470a0187035..833a89f06ae0 100644
--- a/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
+++ b/Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
@@ -18,6 +18,14 @@ Required properties :
 
 - #clock-cells : shall contain 1
 
+The clock enumerators are defined in <dt-bindings/clock/qcom,rpmcc.h>
+and come in pairs: FOO_CLK followed by FOO_A_CLK. The latter clock
+is an "active" clock, which means that the consumer only care that the
+clock is available when the apps CPU subsystem is active, i.e. not
+suspended or in deep idle. If it is important that the clock keeps running
+during system suspend, you need to specify the non-active clock, the one
+not containing *_A_* in the enumerator name.
+
 Example:
 	smd {
 		compatible = "qcom,smd";
-- 
2.9.3

[PATCH 3/5 v2] clk: qcom: Implement RPM clocks for MSM8660/APQ8060

[Linus Walleij]

The RPM clocks were missing for MSM8660/APQ8060. For this to be
completed we need to add a special fixed rate RPM clock that is used
for the PLL4 on these SoCs. The rest of the clocks are pretty
similar to the other supported platforms.

The "active" clock pattern is mirrored in all the clocks. I guess
that the PLL4 that clocks the LPASS is actually never used as
"active only" since the low-power audio subsystem should be left
on when the system suspends, so it can be used as a stand-alone
MP3 player type of device.

As we do not have firmware for the LPASS we will probably only use
this clock when the system is up and running (not suspended) for now,
so that will be using the "active" clock.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Add the small hunk to the clk_rpm_handoff() function that just
  skip over this for the fixed PLL4 clock. This accidentally
  ended up in another patch.
---
 drivers/clk/qcom/clk-rpm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
index df3e5fe8442a..61c67e93bea3 100644
--- a/drivers/clk/qcom/clk-rpm.c
+++ b/drivers/clk/qcom/clk-rpm.c
@@ -56,6 +56,30 @@
 		},							      \
 	}
 
+#define DEFINE_CLK_RPM_FIXED(_platform, _name, _active, r_id, r)	      \
+	static struct clk_rpm _platform##_##_name = {			      \
+		.rpm_clk_id = (r_id),					      \
+		.rate = (r),						      \
+		.hw.init = &(struct clk_init_data){			      \
+			.ops = &clk_rpm_fixed_ops,			      \
+			.name = #_name,					      \
+			.parent_names = (const char *[]){ "pxo_board" },      \
+			.num_parents = 1,				      \
+		},							      \
+	};								      \
+	static struct clk_rpm _platform##_##_active = {			      \
+		.rpm_clk_id = (r_id),					      \
+		.peer = &_platform##_##_name,				      \
+		.active_only = true,					      \
+		.rate = (r),						      \
+		.hw.init = &(struct clk_init_data){			      \
+			.ops = &clk_rpm_fixed_ops,			      \
+			.name = #_active,				      \
+			.parent_names = (const char *[]){ "pxo_board" },      \
+			.num_parents = 1,				      \
+		},							      \
+	}
+
 #define DEFINE_CLK_RPM_PXO_BRANCH(_platform, _name, _active, r_id, r)	      \
 	static struct clk_rpm _platform##_##_active;			      \
 	static struct clk_rpm _platform##_##_name = {			      \
@@ -143,6 +167,13 @@ static int clk_rpm_handoff(struct clk_rpm *r)
 	int ret;
 	u32 value = INT_MAX;
 
+	/*
+	 * The vendor tree simply reads the status for this
+	 * RPM clock.
+	 */
+	if (r->rpm_clk_id == QCOM_RPM_PLL_4)
+		return 0;
+
 	ret = qcom_rpm_write(r->rpm, QCOM_RPM_ACTIVE_STATE,
 			     r->rpm_clk_id, &value, 1);
 	if (ret)
@@ -269,6 +300,32 @@ static void clk_rpm_unprepare(struct clk_hw *hw)
 	mutex_unlock(&rpm_clk_lock);
 }
 
+static int clk_rpm_fixed_prepare(struct clk_hw *hw)
+{
+	struct clk_rpm *r = to_clk_rpm(hw);
+	u32 value = 1;
+	int ret;
+
+	ret = qcom_rpm_write(r->rpm, QCOM_RPM_ACTIVE_STATE,
+			     r->rpm_clk_id, &value, 1);
+	if (!ret)
+		r->enabled = true;
+
+	return ret;
+}
+
+static void clk_rpm_fixed_unprepare(struct clk_hw *hw)
+{
+	struct clk_rpm *r = to_clk_rpm(hw);
+	u32 value = 0;
+	int ret;
+
+	ret = qcom_rpm_write(r->rpm, QCOM_RPM_ACTIVE_STATE,
+			     r->rpm_clk_id, &value, 1);
+	if (!ret)
+		r->enabled = false;
+}
+
 static int clk_rpm_set_rate(struct clk_hw *hw,
 			    unsigned long rate, unsigned long parent_rate)
 {
@@ -333,6 +390,13 @@ static unsigned long clk_rpm_recalc_rate(struct clk_hw *hw,
 	return r->rate;
 }
 
+static const struct clk_ops clk_rpm_fixed_ops = {
+	.prepare	= clk_rpm_fixed_prepare,
+	.unprepare	= clk_rpm_fixed_unprepare,
+	.round_rate	= clk_rpm_round_rate,
+	.recalc_rate	= clk_rpm_recalc_rate,
+};
+
 static const struct clk_ops clk_rpm_ops = {
 	.prepare	= clk_rpm_prepare,
 	.unprepare	= clk_rpm_unprepare,
@@ -348,6 +412,46 @@ static const struct clk_ops clk_rpm_branch_ops = {
 	.recalc_rate	= clk_rpm_recalc_rate,
 };
 
+/* MSM8660/APQ8060 */
+DEFINE_CLK_RPM_FIXED(msm8660, pll4_clk, pll4_a_clk, QCOM_RPM_PLL_4, 540672000);
+DEFINE_CLK_RPM(msm8660, afab_clk, afab_a_clk, QCOM_RPM_APPS_FABRIC_CLK);
+DEFINE_CLK_RPM(msm8660, sfab_clk, sfab_a_clk, QCOM_RPM_SYS_FABRIC_CLK);
+DEFINE_CLK_RPM(msm8660, mmfab_clk, mmfab_a_clk, QCOM_RPM_MM_FABRIC_CLK);
+DEFINE_CLK_RPM(msm8660, daytona_clk, daytona_a_clk, QCOM_RPM_DAYTONA_FABRIC_CLK);
+DEFINE_CLK_RPM(msm8660, sfpb_clk, sfpb_a_clk, QCOM_RPM_SFPB_CLK);
+DEFINE_CLK_RPM(msm8660, cfpb_clk, cfpb_a_clk, QCOM_RPM_CFPB_CLK);
+DEFINE_CLK_RPM(msm8660, mmfpb_clk, mmfpb_a_clk, QCOM_RPM_MMFPB_CLK);
+DEFINE_CLK_RPM(msm8660, smi_clk, smi_a_clk, QCOM_RPM_SMI_CLK);
+DEFINE_CLK_RPM(msm8660, ebi1_clk, ebi1_a_clk, QCOM_RPM_EBI1_CLK);
+
+static struct clk_rpm *msm8660_clks[] = {
+	[RPM_PLL4_CLK] = &msm8660_pll4_clk,
+	[RPM_PLL4_A_CLK] = &msm8660_pll4_a_clk,
+	[RPM_APPS_FABRIC_CLK] = &msm8660_afab_clk,
+	[RPM_APPS_FABRIC_A_CLK] = &msm8660_afab_a_clk,
+	[RPM_SYS_FABRIC_CLK] = &msm8660_sfab_clk,
+	[RPM_SYS_FABRIC_A_CLK] = &msm8660_sfab_a_clk,
+	[RPM_MM_FABRIC_CLK] = &msm8660_mmfab_clk,
+	[RPM_MM_FABRIC_A_CLK] = &msm8660_mmfab_a_clk,
+	[RPM_DAYTONA_FABRIC_CLK] = &msm8660_daytona_clk,
+	[RPM_DAYTONA_FABRIC_A_CLK] = &msm8660_daytona_a_clk,
+	[RPM_SFPB_CLK] = &msm8660_sfpb_clk,
+	[RPM_SFPB_A_CLK] = &msm8660_sfpb_a_clk,
+	[RPM_CFPB_CLK] = &msm8660_cfpb_clk,
+	[RPM_CFPB_A_CLK] = &msm8660_cfpb_a_clk,
+	[RPM_MMFPB_CLK] = &msm8660_mmfpb_clk,
+	[RPM_MMFPB_A_CLK] = &msm8660_mmfpb_a_clk,
+	[RPM_SMI_CLK] = &msm8660_smi_clk,
+	[RPM_SMI_A_CLK] = &msm8660_smi_a_clk,
+	[RPM_EBI1_CLK] = &msm8660_ebi1_clk,
+	[RPM_EBI1_A_CLK] = &msm8660_ebi1_a_clk,
+};
+
+static const struct rpm_clk_desc rpm_clk_msm8660 = {
+	.clks = msm8660_clks,
+	.num_clks = ARRAY_SIZE(msm8660_clks),
+};
+
 /* apq8064 */
 DEFINE_CLK_RPM(apq8064, afab_clk, afab_a_clk, QCOM_RPM_APPS_FABRIC_CLK);
 DEFINE_CLK_RPM(apq8064, cfpb_clk, cfpb_a_clk, QCOM_RPM_CFPB_CLK);
@@ -386,6 +490,8 @@ static const struct rpm_clk_desc rpm_clk_apq8064 = {
 };
 
 static const struct of_device_id rpm_clk_match_table[] = {
+	{ .compatible = "qcom,rpmcc-msm8660", .data = &rpm_clk_msm8660 },
+	{ .compatible = "qcom,rpmcc-apq8060", .data = &rpm_clk_msm8660 },
 	{ .compatible = "qcom,rpmcc-apq8064", .data = &rpm_clk_apq8064 },
 	{ }
 };
-- 
2.9.3

[PATCH 4/5 v2] clk: qcom: Update DT bindings for MSM8660 LCC

[Linus Walleij]

This adds the right compatible string and header for the
MSM8660 LCC and some new defines to the dt-bindings header.

Take this opportunity to spell out the acronym LPASS for
Low-power Audio Subsystem.

Cc: devicetree@vger.kernel.org
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Add Rob's ACK.
---
 .../devicetree/bindings/clock/qcom,lcc.txt         |  5 +--
 include/dt-bindings/clock/qcom,lcc-msm8660.h       | 40 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 include/dt-bindings/clock/qcom,lcc-msm8660.h

diff --git a/Documentation/devicetree/bindings/clock/qcom,lcc.txt b/Documentation/devicetree/bindings/clock/qcom,lcc.txt
index a3c78aa88038..4de51df37f1a 100644
--- a/Documentation/devicetree/bindings/clock/qcom,lcc.txt
+++ b/Documentation/devicetree/bindings/clock/qcom,lcc.txt
@@ -1,10 +1,11 @@
-Qualcomm LPASS Clock & Reset Controller Binding
-------------------------------------------------
+Qualcomm Low-power Audio Subsystem (LPASS) Clock & Reset Controller Binding
+---------------------------------------------------------------------------
 
 Required properties :
 - compatible : shall contain only one of the following:
 
 			"qcom,lcc-msm8960"
+			"qcom,lcc-msm8660"
 			"qcom,lcc-apq8064"
 			"qcom,lcc-ipq8064"
 			"qcom,lcc-mdm9615"
diff --git a/include/dt-bindings/clock/qcom,lcc-msm8660.h b/include/dt-bindings/clock/qcom,lcc-msm8660.h
new file mode 100644
index 000000000000..7cddcbd6b1ee
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,lcc-msm8660.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2017 Linus Walleij <linus.walleij@linaro.org>
+ * Qualcomm MSM8660 Low-power Audio Subsystem (LPASS) Clock Controller
+ * devicetree definitions
+ */
+
+#ifndef _DT_BINDINGS_CLK_LCC_MSM8660_H
+#define _DT_BINDINGS_CLK_LCC_MSM8660_H
+
+#define LPA_PLL0			0
+#define MI2S_OSR_SRC			1
+#define MI2S_OSR_CLK			2
+#define MI2S_DIV_CLK			3
+#define MI2S_BIT_DIV_CLK		4
+#define MI2S_BIT_CLK			5
+#define CODEC_I2S_MIC_OSR_SRC		6
+#define CODEC_I2S_MIC_OSR_CLK		7
+#define CODEC_I2S_MIC_DIV_CLK		8
+#define CODEC_I2S_MIC_BIT_DIV_CLK	9
+#define CODEC_I2S_MIC_BIT_CLK		10
+#define SPARE_I2S_MIC_OSR_SRC		11
+#define SPARE_I2S_MIC_OSR_CLK		12
+#define SPARE_I2S_MIC_DIV_CLK		13
+#define SPARE_I2S_MIC_BIT_DIV_CLK	14
+#define SPARE_I2S_MIC_BIT_CLK		15
+#define CODEC_I2S_SPKR_OSR_SRC		16
+#define CODEC_I2S_SPKR_OSR_CLK		17
+#define CODEC_I2S_SPKR_DIV_CLK		18
+#define CODEC_I2S_SPKR_BIT_DIV_CLK	19
+#define CODEC_I2S_SPKR_BIT_CLK		20
+#define SPARE_I2S_SPKR_OSR_SRC		21
+#define SPARE_I2S_SPKR_OSR_CLK		22
+#define SPARE_I2S_SPKR_DIV_CLK		23
+#define SPARE_I2S_SPKR_BIT_DIV_CLK	24
+#define SPARE_I2S_SPKR_BIT_CLK		25
+#define PCM_SRC				26
+#define PCM_CLK_OUT			27
+#define PCM_CLK				28
+
+#endif
-- 
2.9.3

[PATCH 5/5 v2] clk: qcom: Add support for MSM8660 LCC

[Linus Walleij]

This adds support for the MSM8660 Low-power audio
subsystem (LPASS) clock controller (LCC). This is nice when
you want to have audio from the system.

We currently only support using the PLL4 (which is an RPM
clock) as the parent, however the LPASS has its own PLL named
LPA_PLL0 that we can experiment with enabling later on, so
for this reason the code contains a few hints on how to enable
the LPA_PLL0.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- No changes.
---
 drivers/clk/qcom/Kconfig       |   9 +
 drivers/clk/qcom/Makefile      |   1 +
 drivers/clk/qcom/lcc-msm8660.c | 418 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 428 insertions(+)
 create mode 100644 drivers/clk/qcom/lcc-msm8660.c

diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 5fb8d7430908..d657d94d372f 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -90,6 +90,15 @@ config MSM_GCC_8660
 	  Say Y if you want to use peripheral devices such as UART, SPI,
 	  i2c, USB, SD/eMMC, etc.
 
+config MSM_LCC_8660
+	tristate "MSM8660 LPASS Clock Controller"
+	select MSM_GCC_8660
+	depends on COMMON_CLK_QCOM
+	help
+	  Support for the Low-power Audio Subsystem (LPASS) clock controller
+	  on MSM8660 devices.
+	  Say Y if you want to use audio devices such as I2S and PCM.
+
 config MSM_GCC_8916
 	tristate "MSM8916 Global Clock Controller"
 	select QCOM_GDSC
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index 1c3e222b917b..d6d5ab178522 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
 obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
 obj-$(CONFIG_MDM_LCC_9615) += lcc-mdm9615.o
 obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
+obj-$(CONFIG_MSM_LCC_8660) += lcc-msm8660.o
 obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
 obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
 obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
diff --git a/drivers/clk/qcom/lcc-msm8660.c b/drivers/clk/qcom/lcc-msm8660.c
new file mode 100644
index 000000000000..73dc8ede6a2a
--- /dev/null
+++ b/drivers/clk/qcom/lcc-msm8660.c
@@ -0,0 +1,418 @@
+/*
+ * Qualcomm MSM8660/APQ8060 Low-power Audio Subsystem (LPASS) Clock Controller
+ * Copyright (c) 2017 Linus Walleij <linus.walleij@linaro.org>
+ *
+ * Based on a copy of the IPQ806x driver
+ * (C) 2014 Rajendra Nayak
+ * and portions of the MDM9615 driver
+ * (C) 2014 Neil Armstrong.
+ */
+
+#include <linux/kernel.h>
+#include <linux/bitops.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,lcc-msm8660.h>
+
+#include "common.h"
+#include "clk-regmap.h"
+#include "clk-pll.h"
+#include "clk-rcg.h"
+#include "clk-branch.h"
+#include "clk-regmap-divider.h"
+#include "clk-regmap-mux.h"
+
+/*
+ * The vendor tree calls this "PLL0" but we are going to refer to it as
+ * LPA_PLL0 so as not to confuse it with the PLL0 on the GCC.
+ */
+static struct clk_pll lpa_pll0 = {
+	.l_reg = 0x4,
+	.m_reg = 0x8,
+	.n_reg = 0xc,
+	.config_reg = 0x14,
+	.mode_reg = 0x0,
+	.status_reg = 0x18,
+	.status_bit = 16,
+	.clkr.hw.init = &(struct clk_init_data){
+		.name = "lpa_pll0",
+		.parent_names = (const char *[]){ "pxo" },
+		.num_parents = 1,
+		.ops = &clk_pll_ops,
+	},
+};
+
+/*
+ * The l/m/n values were read out of the registers after a cold boot.
+ * The config register read 00c22080 and the mode register 0x00000007.
+ */
+static const struct pll_config lpa_pll0_config = {
+	.l = 0xf,
+	.m = 0x1c,
+	.n = 0x465,
+	.vco_val = BIT(17),
+	.vco_mask = BIT(17) | BIT(16),
+	.pre_div_val = 0x0,
+	.pre_div_mask = BIT(19),
+	.post_div_val = 0x0,
+	.post_div_mask = BIT(21) | BIT(20),
+	.mn_ena_mask = BIT(22),
+	.main_output_mask = BIT(23),
+};
+
+enum {
+	P_PXO,
+	P_CXO,
+	P_PLL4_LPA_PLL0,
+	P_GND,
+};
+
+/* The vendor code uses PLL4 as parent everywhere */
+static const struct parent_map lcc_parent_map[] = {
+	{ P_PXO, 0 },
+	{ P_CXO, 1 },
+	/* Select RPM PLL4, but also used for selecting LPA PLL0 */
+	{ P_PLL4_LPA_PLL0, 2 },
+	/* Will just ground the line */
+	{ P_GND, 6 },
+};
+
+static const char * const lcc_parent_tbl[] = {
+	"pxo",
+	"cxo",
+	/*
+	 * PLL4 is an RPM clock on MSM8660/APQ8060, set to "pll4" for this
+	 * If we enable and mux in the LPA_PLL0 on this platform, we can
+	 * set this to "lpa_pll0" instead
+	 */
+	"pll4_clk",
+	"gnd", /* This is a very inactive parent */
+};
+
+/*
+ * This table is evidently for using PLL4 as parent, if we start using
+ * LPA_PLL0 we need to provide a second table.
+ */
+static struct freq_tbl clk_tbl_aif_osr_pll4[] = {
+	{   768000, P_PLL4_LPA_PLL0, 4,  1, 176 },
+	{  1024000, P_PLL4_LPA_PLL0, 4,  1, 132 },
+	{  1536000, P_PLL4_LPA_PLL0, 4,  1,  88 },
+	{  2048000, P_PLL4_LPA_PLL0, 4,  1,  66 },
+	{  3072000, P_PLL4_LPA_PLL0, 4,  1,  44 },
+	{  4096000, P_PLL4_LPA_PLL0, 4,  1,  33 },
+	{  6144000, P_PLL4_LPA_PLL0, 4,  1,  22 },
+	{  8192000, P_PLL4_LPA_PLL0, 2,  1,  33 },
+	{ 12288000, P_PLL4_LPA_PLL0, 4,  1,  11 },
+	{ 24576000, P_PLL4_LPA_PLL0, 2,  1,  11 },
+	{ 27000000, P_PXO,           1,  0,   0 },
+	{ }
+};
+
+/*
+ * This macro is modified from lcc-mdm9516.c, it's used for all the
+ * AIF clocks, all of them have an OSR clock and a bit clock derived
+ * from the OSR clock.
+ *
+ * These clocks differ from many other platforms by using
+ * BRANCH_HALT_DELAY for the *_bit_div_clk
+ */
+#define CLK_AIF_OSR_DIV(prefix, _ns, _md, _hr)			\
+static struct clk_rcg prefix##_osr_src = {			\
+	.ns_reg = _ns,						\
+	.md_reg = _md,						\
+	.mn = {							\
+		.mnctr_en_bit = 8,				\
+		.mnctr_reset_bit = 7,				\
+		.mnctr_mode_shift = 5,				\
+		.n_val_shift = 24,				\
+		.m_val_shift = 8,				\
+		.width = 8,					\
+	},							\
+	.p = {							\
+		.pre_div_shift = 3,				\
+		.pre_div_width = 2,				\
+	},							\
+	.s = {							\
+		.src_sel_shift = 0,				\
+		.parent_map = lcc_parent_map,			\
+	},							\
+	.freq_tbl = clk_tbl_aif_osr_pll4,			\
+	.clkr = {						\
+		.enable_reg = _ns,				\
+		.enable_mask = BIT(9),				\
+		.hw.init = &(struct clk_init_data){		\
+			.name = #prefix "_osr_src",		\
+			.parent_names = lcc_parent_tbl,		\
+			.num_parents = 4,			\
+			.ops = &clk_rcg_ops,			\
+			.flags = CLK_SET_RATE_GATE,		\
+		},						\
+	},							\
+};								\
+								\
+static const char * const lcc_##prefix##_parents[] = {		\
+	#prefix "_osr_src",					\
+};								\
+								\
+static struct clk_branch prefix##_osr_clk = {			\
+	.halt_reg = _hr,					\
+	.halt_bit = 1,						\
+	.halt_check = BRANCH_HALT_ENABLE,			\
+	.clkr = {						\
+		.enable_reg = _ns,				\
+		.enable_mask = BIT(17),				\
+		.hw.init = &(struct clk_init_data){		\
+			.name = #prefix "_osr_clk",		\
+			.parent_names = lcc_##prefix##_parents,	\
+			.num_parents = 1,			\
+			.ops = &clk_branch_ops,			\
+			.flags = CLK_SET_RATE_PARENT,		\
+		},						\
+	},							\
+};								\
+								\
+static struct clk_regmap_div prefix##_div_clk = {		\
+	.reg = _ns,						\
+	.shift = 10,						\
+	.width = 4,						\
+	.clkr = {						\
+		.hw.init = &(struct clk_init_data){		\
+			.name = #prefix "_div_clk",		\
+			.parent_names = lcc_##prefix##_parents,	\
+			.num_parents = 1,			\
+			.ops = &clk_regmap_div_ops,		\
+		},						\
+	},							\
+};								\
+								\
+static struct clk_branch prefix##_bit_div_clk = {		\
+	.halt_reg = _hr,					\
+	.halt_bit = 0,						\
+	.halt_check = BRANCH_HALT_DELAY,			\
+	.clkr = {						\
+		.enable_reg = _ns,				\
+		.enable_mask = BIT(15),				\
+		.hw.init = &(struct clk_init_data){		\
+			.name = #prefix "_bit_div_clk",		\
+			.parent_names = (const char *[]){	\
+				#prefix "_div_clk"		\
+			},					\
+			.num_parents = 1,			\
+			.ops = &clk_branch_ops,			\
+			.flags = CLK_SET_RATE_PARENT,		\
+		},						\
+	},							\
+};								\
+								\
+static struct clk_regmap_mux prefix##_bit_clk = {		\
+	.reg = _ns,						\
+	.shift = 14,						\
+	.width = 1,						\
+	.clkr = {						\
+		.hw.init = &(struct clk_init_data){		\
+			.name = #prefix "_bit_clk",		\
+			.parent_names = (const char *[]){	\
+				#prefix "_bit_div_clk",		\
+				#prefix "_codec_clk",		\
+			},					\
+			.num_parents = 2,			\
+			.ops = &clk_regmap_mux_closest_ops,	\
+			.flags = CLK_SET_RATE_PARENT,		\
+		},						\
+	},							\
+}
+
+CLK_AIF_OSR_DIV(mi2s, 0x48, 0x4c, 0x50);
+CLK_AIF_OSR_DIV(codec_i2s_mic, 0x60, 0x64, 0x68);
+CLK_AIF_OSR_DIV(spare_i2s_mic, 0x78, 0x7c, 0x80);
+CLK_AIF_OSR_DIV(codec_i2s_spkr, 0x6c, 0x70, 0x74);
+CLK_AIF_OSR_DIV(spare_i2s_spkr, 0x84, 0x88, 0x8c);
+
+/*
+ * PCM clock
+ * This table is evidently for using PLL4 as parent, if we start using
+ * LPA_PLL0 we need to provide a second table.
+ */
+static struct freq_tbl clk_tbl_pcm_pll4[] = {
+	{   512000, P_PLL4_LPA_PLL0, 4, 1, 264 },
+	{   768000, P_PLL4_LPA_PLL0, 4, 1, 176 },
+	{  1024000, P_PLL4_LPA_PLL0, 4, 1, 132 },
+	{  1536000, P_PLL4_LPA_PLL0, 4, 1,  88 },
+	{  2048000, P_PLL4_LPA_PLL0, 4, 1,  66 },
+	{  3072000, P_PLL4_LPA_PLL0, 4, 1,  44 },
+	{  4096000, P_PLL4_LPA_PLL0, 4, 1,  33 },
+	{  6144000, P_PLL4_LPA_PLL0, 4, 1,  22 },
+	{  8192000, P_PLL4_LPA_PLL0, 2, 1,  33 },
+	{ 12288000, P_PLL4_LPA_PLL0, 4, 1,  11 },
+	{ 24580000, P_PLL4_LPA_PLL0, 2, 1,  11 },
+	{ 27000000, P_PXO,           1, 0,   0 },
+	{ },
+};
+
+static struct clk_rcg pcm_src = {
+	.ns_reg = 0x54,
+	.md_reg = 0x58,
+	.mn = {
+		.mnctr_en_bit = 8,
+		.mnctr_reset_bit = 7,
+		.mnctr_mode_shift = 5,
+		.n_val_shift = 16,
+		.m_val_shift = 16,
+		.width = 16,
+	},
+	.p = {
+		.pre_div_shift = 3,
+		.pre_div_width = 2,
+	},
+	.s = {
+		.src_sel_shift = 0,
+		.parent_map = lcc_parent_map,
+	},
+	.freq_tbl = clk_tbl_pcm_pll4,
+	.clkr = {
+		.enable_reg = 0x54,
+		.enable_mask = BIT(9),
+		.hw.init = &(struct clk_init_data){
+			.name = "pcm_src",
+			.parent_names = lcc_parent_tbl,
+			.num_parents = 4,
+			.ops = &clk_rcg_ops,
+			.flags = CLK_SET_RATE_GATE,
+		},
+	},
+};
+
+static struct clk_branch pcm_clk_out = {
+	.halt_reg = 0x5c,
+	.halt_bit = 0,
+	.halt_check = BRANCH_HALT_ENABLE,
+	.clkr = {
+		.enable_reg = 0x54,
+		.enable_mask = BIT(11),
+		.hw.init = &(struct clk_init_data){
+			.name = "pcm_clk_out",
+			.parent_names = (const char *[]){ "pcm_src" },
+			.num_parents = 1,
+			.ops = &clk_branch_ops,
+			.flags = CLK_SET_RATE_PARENT,
+		},
+	},
+};
+
+static struct clk_regmap_mux pcm_clk = {
+	.reg = 0x54,
+	.shift = 10,
+	.width = 1,
+	.clkr = {
+		.hw.init = &(struct clk_init_data){
+			.name = "pcm_clk",
+			.parent_names = (const char *[]){
+				"pcm_clk_out",
+				"pcm_codec_clk",
+			},
+			.num_parents = 2,
+			.ops = &clk_regmap_mux_closest_ops,
+			.flags = CLK_SET_RATE_PARENT,
+		},
+	},
+};
+
+static struct clk_regmap *lcc_msm8660_clks[] = {
+	[LPA_PLL0] = &lpa_pll0.clkr,
+	[MI2S_OSR_SRC] = &mi2s_osr_src.clkr,
+	[MI2S_OSR_CLK] = &mi2s_osr_clk.clkr,
+	[MI2S_DIV_CLK] = &mi2s_div_clk.clkr,
+	[MI2S_BIT_DIV_CLK] = &mi2s_bit_div_clk.clkr,
+	[MI2S_BIT_CLK] = &mi2s_bit_clk.clkr,
+	[CODEC_I2S_MIC_OSR_SRC] = &codec_i2s_mic_osr_src.clkr,
+	[CODEC_I2S_MIC_OSR_CLK] = &codec_i2s_mic_osr_clk.clkr,
+	[CODEC_I2S_MIC_DIV_CLK] = &codec_i2s_mic_div_clk.clkr,
+	[CODEC_I2S_MIC_BIT_DIV_CLK] = &codec_i2s_mic_bit_div_clk.clkr,
+	[CODEC_I2S_MIC_BIT_CLK] = &codec_i2s_mic_bit_clk.clkr,
+	[SPARE_I2S_MIC_OSR_SRC] = &spare_i2s_mic_osr_src.clkr,
+	[SPARE_I2S_MIC_OSR_CLK] = &spare_i2s_mic_osr_clk.clkr,
+	[SPARE_I2S_MIC_DIV_CLK] = &spare_i2s_mic_div_clk.clkr,
+	[SPARE_I2S_MIC_BIT_DIV_CLK] = &spare_i2s_mic_bit_div_clk.clkr,
+	[SPARE_I2S_MIC_BIT_CLK] = &spare_i2s_mic_bit_clk.clkr,
+	[CODEC_I2S_SPKR_OSR_SRC] = &codec_i2s_spkr_osr_src.clkr,
+	[CODEC_I2S_SPKR_OSR_CLK] = &codec_i2s_spkr_osr_clk.clkr,
+	[CODEC_I2S_SPKR_DIV_CLK] = &codec_i2s_spkr_div_clk.clkr,
+	[CODEC_I2S_SPKR_BIT_DIV_CLK] = &codec_i2s_spkr_bit_div_clk.clkr,
+	[CODEC_I2S_SPKR_BIT_CLK] = &codec_i2s_spkr_bit_clk.clkr,
+	[SPARE_I2S_SPKR_OSR_SRC] = &spare_i2s_spkr_osr_src.clkr,
+	[SPARE_I2S_SPKR_OSR_CLK] = &spare_i2s_spkr_osr_clk.clkr,
+	[SPARE_I2S_SPKR_DIV_CLK] = &spare_i2s_spkr_div_clk.clkr,
+	[SPARE_I2S_SPKR_BIT_DIV_CLK] = &spare_i2s_spkr_bit_div_clk.clkr,
+	[SPARE_I2S_SPKR_BIT_CLK] = &spare_i2s_spkr_bit_clk.clkr,
+	[PCM_SRC] = &pcm_src.clkr,
+	[PCM_CLK_OUT] = &pcm_clk_out.clkr,
+	[PCM_CLK] = &pcm_clk.clkr,
+};
+
+static const struct regmap_config lcc_msm8660_regmap_config = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= 0xfc,
+	.fast_io	= true,
+};
+
+static const struct qcom_cc_desc lcc_msm8660_desc = {
+	.config = &lcc_msm8660_regmap_config,
+	.clks = lcc_msm8660_clks,
+	.num_clks = ARRAY_SIZE(lcc_msm8660_clks),
+};
+
+static const struct of_device_id lcc_msm8660_match_table[] = {
+	{ .compatible = "qcom,lcc-msm8660" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, lcc_msm8660_match_table);
+
+static int lcc_msm8660_probe(struct platform_device *pdev)
+{
+	u32 val;
+	struct regmap *regmap;
+
+	regmap = qcom_cc_map(pdev, &lcc_msm8660_desc);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	/* Configure the rate of LPA_PLL0 if the bootloader hasn't already */
+	regmap_read(regmap, 0x0, &val);
+	if (!val) {
+		dev_info(&pdev->dev, "configuring LPA_PLL0\n");
+		clk_pll_configure_sr(&lpa_pll0, regmap, &lpa_pll0_config, true);
+	} else {
+		dev_info(&pdev->dev,
+			 "LPA_PLL0 already configured\n");
+	}
+
+	/*
+	 * Enable LPA_PLL0 source on the LPASS Primary PLL Mux. Incidentally
+	 * this is set to 0x00000001 at boot.
+	 * 0x01 = LPA_PLL0
+	 */
+	regmap_write(regmap, 0xc4, 0x1);
+
+	return qcom_cc_really_probe(pdev, &lcc_msm8660_desc, regmap);
+}
+
+static struct platform_driver lcc_msm8660_driver = {
+	.probe		= lcc_msm8660_probe,
+	.driver		= {
+		.name	= "lcc-msm8660",
+		.of_match_table = lcc_msm8660_match_table,
+	},
+};
+module_platform_driver(lcc_msm8660_driver);
+
+MODULE_DESCRIPTION("QCOM LCC MSM8660 Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:lcc-msm8660");
-- 
2.9.3

Re: [PATCH 2/5 v2] clk: qcom: Elaborate on "active" clocks in the RPM clock bindings

[Rob Herring]

On Wed, Apr 19, 2017 at 11:13:23AM +0200, Linus Walleij wrote:
> The concept of "active" clocks is just explained in a bried comment in the
> device driver, let's explain it a bit more in the device tree bindings
> so everyone understands this.
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> ChangeLog v1->v2:
> - Reword slighty in accordance with Stephens feedback.
> ---
>  Documentation/devicetree/bindings/clock/qcom,rpmcc.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)

Despite my objections, you're just documenting what is already there.

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5 v2] clk: qcom: Elaborate on "active" clocks in the RPM clock bindings

[Rob Herring]

On Wed, Apr 19, 2017 at 11:13:23AM +0200, Linus Walleij wrote:
> The concept of "active" clocks is just explained in a bried comment in the
> device driver, let's explain it a bit more in the device tree bindings
> so everyone understands this.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Reword slighty in accordance with Stephens feedback.
> ---
>  Documentation/devicetree/bindings/clock/qcom,rpmcc.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)

Despite my objections, you're just documenting what is already there.

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Linus Walleij]

On Wed, Apr 19, 2017 at 11:13 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:

> These compatible strings need to be added to extend support
> for the RPM CC to cover MSM8660/APQ8060. We also need to add
> enumberators to the include file for a few clocks that were
> missing.
>
> Cc: devicetree@vger.kernel.org
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Collect Rob's ACK

Ping on this patch set. It's been sitting for a month and was
not changed from v1 either, if you want me to change something
it'd be good to know.

Yours,
Linus Walleij

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Linus Walleij]

On Wed, May 17, 2017 at 9:23 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Apr 19, 2017 at 11:13 AM, Linus Walleij
> <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
>> These compatible strings need to be added to extend support
>> for the RPM CC to cover MSM8660/APQ8060. We also need to add
>> enumberators to the include file for a few clocks that were
>> missing.
>>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> ChangeLog v1->v2:
>> - Collect Rob's ACK
>
> Ping on this patch set. It's been sitting for a month and was
> not changed from v1 either, if you want me to change something
> it'd be good to know.

Ping on this again.

I am sorry if you guys are swamped, I just don't know what to do,
clock drivers are starting to become the bottleneck for everything
I do. I bet the ARM SoC maintainers must be noticing it too.

Unfortunately I am already managing two subsystems and cannot
volunteer to help out with a third, I would blow a fuse.

But it seems the CLK subsystem could use some co-maintainers?

Peter de Schrijver is a big authority in all things clocking (I clearly
remember him discussing the subject a lot at conferences with lots
of good points) so I would ask him to help, but I don't know how busy
Peter may be with nVidia business etc these days...

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Linus Walleij]

On Wed, May 17, 2017 at 9:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Apr 19, 2017 at 11:13 AM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
>
>> These compatible strings need to be added to extend support
>> for the RPM CC to cover MSM8660/APQ8060. We also need to add
>> enumberators to the include file for a few clocks that were
>> missing.
>>
>> Cc: devicetree@vger.kernel.org
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> ChangeLog v1->v2:
>> - Collect Rob's ACK
>
> Ping on this patch set. It's been sitting for a month and was
> not changed from v1 either, if you want me to change something
> it'd be good to know.

Ping on this again.

I am sorry if you guys are swamped, I just don't know what to do,
clock drivers are starting to become the bottleneck for everything
I do. I bet the ARM SoC maintainers must be noticing it too.

Unfortunately I am already managing two subsystems and cannot
volunteer to help out with a third, I would blow a fuse.

But it seems the CLK subsystem could use some co-maintainers?

Peter de Schrijver is a big authority in all things clocking (I clearly
remember him discussing the subject a lot at conferences with lots
of good points) so I would ask him to help, but I don't know how busy
Peter may be with nVidia business etc these days...

Yours,
Linus Walleij

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Peter De Schrijver]

On Wed, May 24, 2017 at 11:16:22AM +0200, Linus Walleij wrote:
> On Wed, May 17, 2017 at 9:23 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> > On Wed, Apr 19, 2017 at 11:13 AM, Linus Walleij
> > <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >
> >> These compatible strings need to be added to extend support
> >> for the RPM CC to cover MSM8660/APQ8060. We also need to add
> >> enumberators to the include file for a few clocks that were
> >> missing.
> >>
> >> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> >> ---
> >> ChangeLog v1->v2:
> >> - Collect Rob's ACK
> >
> > Ping on this patch set. It's been sitting for a month and was
> > not changed from v1 either, if you want me to change something
> > it'd be good to know.
> 
> Ping on this again.
> 
> I am sorry if you guys are swamped, I just don't know what to do,
> clock drivers are starting to become the bottleneck for everything
> I do. I bet the ARM SoC maintainers must be noticing it too.
> 
> Unfortunately I am already managing two subsystems and cannot
> volunteer to help out with a third, I would blow a fuse.
> 
> But it seems the CLK subsystem could use some co-maintainers?
> 
> Peter de Schrijver is a big authority in all things clocking (I clearly
> remember him discussing the subject a lot at conferences with lots
> of good points) so I would ask him to help, but I don't know how busy
> Peter may be with nVidia business etc these days...
> 

I'm usually rather busy yes, but I could try to find some time for this if
asked for.

Peter.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Peter De Schrijver]

On Wed, May 24, 2017 at 11:16:22AM +0200, Linus Walleij wrote:
> On Wed, May 17, 2017 at 9:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Apr 19, 2017 at 11:13 AM, Linus Walleij
> > <linus.walleij@linaro.org> wrote:
> >
> >> These compatible strings need to be added to extend support
> >> for the RPM CC to cover MSM8660/APQ8060. We also need to add
> >> enumberators to the include file for a few clocks that were
> >> missing.
> >>
> >> Cc: devicetree@vger.kernel.org
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> ---
> >> ChangeLog v1->v2:
> >> - Collect Rob's ACK
> >
> > Ping on this patch set. It's been sitting for a month and was
> > not changed from v1 either, if you want me to change something
> > it'd be good to know.
> 
> Ping on this again.
> 
> I am sorry if you guys are swamped, I just don't know what to do,
> clock drivers are starting to become the bottleneck for everything
> I do. I bet the ARM SoC maintainers must be noticing it too.
> 
> Unfortunately I am already managing two subsystems and cannot
> volunteer to help out with a third, I would blow a fuse.
> 
> But it seems the CLK subsystem could use some co-maintainers?
> 
> Peter de Schrijver is a big authority in all things clocking (I clearly
> remember him discussing the subject a lot at conferences with lots
> of good points) so I would ask him to help, but I don't know how busy
> Peter may be with nVidia business etc these days...
> 

I'm usually rather busy yes, but I could try to find some time for this if
asked for.

Peter.

Re: [PATCH 3/5 v2] clk: qcom: Implement RPM clocks for MSM8660/APQ8060

[Bjorn Andersson]

On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:

> The RPM clocks were missing for MSM8660/APQ8060. For this to be
> completed we need to add a special fixed rate RPM clock that is used
> for the PLL4 on these SoCs. The rest of the clocks are pretty
> similar to the other supported platforms.
> 
> The "active" clock pattern is mirrored in all the clocks. I guess
> that the PLL4 that clocks the LPASS is actually never used as
> "active only" since the low-power audio subsystem should be left
> on when the system suspends, so it can be used as a stand-alone
> MP3 player type of device.
> 
> As we do not have firmware for the LPASS we will probably only use
> this clock when the system is up and running (not suspended) for now,
> so that will be using the "active" clock.
> 

Note that "active" vs "sleep" is not related to the Linux suspend state,
but rather the CPU idle state; at the bottom of the CPU idle path the
RPM will react and reconfigure resources to their sleep state (if one is
configured) and then reconfigured based on the active state before
returning from the idle.

The PLL4 seems to be enabled only on behalf of the booting LPASS Hexagon
- which will cast its own vote once its booted - and as such we only
configure the active state (meaning both states will have same
configuration).  The result is that PLL4 will be on from prepare() to
unprepare() regardless of what the application CPU does.

> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Add the small hunk to the clk_rpm_handoff() function that just
>   skip over this for the fixed PLL4 clock. This accidentally
>   ended up in another patch.
> ---
>  drivers/clk/qcom/clk-rpm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 106 insertions(+)
> 
> diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> index df3e5fe8442a..61c67e93bea3 100644
> --- a/drivers/clk/qcom/clk-rpm.c
> +++ b/drivers/clk/qcom/clk-rpm.c
> @@ -56,6 +56,30 @@
>  		},							      \
>  	}
>  
> +#define DEFINE_CLK_RPM_FIXED(_platform, _name, _active, r_id, r)	      \

Is there a reason why you don't use DEFINE_CLK_RPM_PXO_BRANCH() for
PLL4?

Looking at the downstream code PLL4 is explicitly handled differently
and is only exposing one state. So if you're seeing issues with reusing
the PXO_BRANCH() I think you should slim this down further and only
register a single clk_rpm from this.

> +	static struct clk_rpm _platform##_##_name = {			      \
> +		.rpm_clk_id = (r_id),					      \
> +		.rate = (r),						      \
> +		.hw.init = &(struct clk_init_data){			      \
> +			.ops = &clk_rpm_fixed_ops,			      \
> +			.name = #_name,					      \
> +			.parent_names = (const char *[]){ "pxo_board" },      \
> +			.num_parents = 1,				      \
> +		},							      \
> +	};								      \
> +	static struct clk_rpm _platform##_##_active = {			      \
> +		.rpm_clk_id = (r_id),					      \
> +		.peer = &_platform##_##_name,				      \
> +		.active_only = true,					      \
> +		.rate = (r),						      \
> +		.hw.init = &(struct clk_init_data){			      \
> +			.ops = &clk_rpm_fixed_ops,			      \
> +			.name = #_active,				      \
> +			.parent_names = (const char *[]){ "pxo_board" },      \
> +			.num_parents = 1,				      \
> +		},							      \
> +	}
> +
[..]
> @@ -348,6 +412,46 @@ static const struct clk_ops clk_rpm_branch_ops = {
>  	.recalc_rate	= clk_rpm_recalc_rate,
>  };
>  
> +/* MSM8660/APQ8060 */
> +DEFINE_CLK_RPM_FIXED(msm8660, pll4_clk, pll4_a_clk, QCOM_RPM_PLL_4, 540672000);

My OCD wants this at the bottom of the list...

> +DEFINE_CLK_RPM(msm8660, afab_clk, afab_a_clk, QCOM_RPM_APPS_FABRIC_CLK);
> +DEFINE_CLK_RPM(msm8660, sfab_clk, sfab_a_clk, QCOM_RPM_SYS_FABRIC_CLK);
> +DEFINE_CLK_RPM(msm8660, mmfab_clk, mmfab_a_clk, QCOM_RPM_MM_FABRIC_CLK);
> +DEFINE_CLK_RPM(msm8660, daytona_clk, daytona_a_clk, QCOM_RPM_DAYTONA_FABRIC_CLK);
> +DEFINE_CLK_RPM(msm8660, sfpb_clk, sfpb_a_clk, QCOM_RPM_SFPB_CLK);
> +DEFINE_CLK_RPM(msm8660, cfpb_clk, cfpb_a_clk, QCOM_RPM_CFPB_CLK);
> +DEFINE_CLK_RPM(msm8660, mmfpb_clk, mmfpb_a_clk, QCOM_RPM_MMFPB_CLK);
> +DEFINE_CLK_RPM(msm8660, smi_clk, smi_a_clk, QCOM_RPM_SMI_CLK);
> +DEFINE_CLK_RPM(msm8660, ebi1_clk, ebi1_a_clk, QCOM_RPM_EBI1_CLK);

Regards,
Bjorn

Re: [PATCH 5/5 v2] clk: qcom: Add support for MSM8660 LCC

[Bjorn Andersson]

On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:
> diff --git a/drivers/clk/qcom/lcc-msm8660.c b/drivers/clk/qcom/lcc-msm8660.c
[..]
> +/* The vendor code uses PLL4 as parent everywhere */
> +static const struct parent_map lcc_parent_map[] = {
> +	{ P_PXO, 0 },
> +	{ P_CXO, 1 },
> +	/* Select RPM PLL4, but also used for selecting LPA PLL0 */
> +	{ P_PLL4_LPA_PLL0, 2 },
> +	/* Will just ground the line */
> +	{ P_GND, 6 },
> +};
> +
> +static const char * const lcc_parent_tbl[] = {
> +	"pxo",
> +	"cxo",
> +	/*
> +	 * PLL4 is an RPM clock on MSM8660/APQ8060, set to "pll4" for this
> +	 * If we enable and mux in the LPA_PLL0 on this platform, we can
> +	 * set this to "lpa_pll0" instead
> +	 */
> +	"pll4_clk",
> +	"gnd", /* This is a very inactive parent */
> +};
> +
> +/*
> + * This table is evidently for using PLL4 as parent, if we start using
> + * LPA_PLL0 we need to provide a second table.
> + */

Aren't you muxing in LPA_PLL0 as source instead of PLL4 at the bottom of
probe()? And as you hard code that selector, shouldn't the parent table
reference lpa_pll0?

> +static struct freq_tbl clk_tbl_aif_osr_pll4[] = {
> +	{   768000, P_PLL4_LPA_PLL0, 4,  1, 176 },
> +	{  1024000, P_PLL4_LPA_PLL0, 4,  1, 132 },
> +	{  1536000, P_PLL4_LPA_PLL0, 4,  1,  88 },
> +	{  2048000, P_PLL4_LPA_PLL0, 4,  1,  66 },
> +	{  3072000, P_PLL4_LPA_PLL0, 4,  1,  44 },
> +	{  4096000, P_PLL4_LPA_PLL0, 4,  1,  33 },
> +	{  6144000, P_PLL4_LPA_PLL0, 4,  1,  22 },
> +	{  8192000, P_PLL4_LPA_PLL0, 2,  1,  33 },
> +	{ 12288000, P_PLL4_LPA_PLL0, 4,  1,  11 },
> +	{ 24576000, P_PLL4_LPA_PLL0, 2,  1,  11 },
> +	{ 27000000, P_PXO,           1,  0,   0 },
> +	{ }
> +};
> +
[..]
> +static int lcc_msm8660_probe(struct platform_device *pdev)
> +{
[..]
> +	/*
> +	 * Enable LPA_PLL0 source on the LPASS Primary PLL Mux. Incidentally
> +	 * this is set to 0x00000001 at boot.
> +	 * 0x01 = LPA_PLL0
> +	 */
> +	regmap_write(regmap, 0xc4, 0x1);

Regards,
Bjorn

Re: [PATCH 5/5 v2] clk: qcom: Add support for MSM8660 LCC

[Linus Walleij]

On Sat, May 27, 2017 at 10:19 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:
>> diff --git a/drivers/clk/qcom/lcc-msm8660.c b/drivers/clk/qcom/lcc-msm8660.c
> [..]
>> +/* The vendor code uses PLL4 as parent everywhere */
>> +static const struct parent_map lcc_parent_map[] = {
>> +     { P_PXO, 0 },
>> +     { P_CXO, 1 },
>> +     /* Select RPM PLL4, but also used for selecting LPA PLL0 */
>> +     { P_PLL4_LPA_PLL0, 2 },
>> +     /* Will just ground the line */
>> +     { P_GND, 6 },
>> +};
>> +
>> +static const char * const lcc_parent_tbl[] = {
>> +     "pxo",
>> +     "cxo",
>> +     /*
>> +      * PLL4 is an RPM clock on MSM8660/APQ8060, set to "pll4" for this
>> +      * If we enable and mux in the LPA_PLL0 on this platform, we can
>> +      * set this to "lpa_pll0" instead
>> +      */
>> +     "pll4_clk",
>> +     "gnd", /* This is a very inactive parent */
>> +};
>> +
>> +/*
>> + * This table is evidently for using PLL4 as parent, if we start using
>> + * LPA_PLL0 we need to provide a second table.
>> + */
>
> Aren't you muxing in LPA_PLL0 as source instead of PLL4 at the bottom of
> probe()? And as you hard code that selector, shouldn't the parent table
> reference lpa_pll0?

I think it's just a naming problem.

I'm actually using:

>> +     {   768000, P_PLL4_LPA_PLL0, 4,  1, 176 },

Because as far as I can tell, RPM clock PLL4 and LPA_PLL0 is the
same thing, just different names.

I don't think the 8660 has its own LPA PLL, it's just the layers of
confusion in the code that make it seem like so.

Or maybe someone was in the planning stages of adding the LPA PLL
to the hardware and never got there. So some code confusingly
refers to that...

I would really like to see how it actually works.

BTW: do you have a pointer to a vendor tree for a Sony thing using
the MSM8660 or APQ8060 that I can look at? I need more known
working code to inspect.

Yours,
Linus Walleij

Re: [PATCH 5/5 v2] clk: qcom: Add support for MSM8660 LCC

[Bjorn Andersson]

On Mon 29 May 05:23 PDT 2017, Linus Walleij wrote:

> On Sat, May 27, 2017 at 10:19 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:
> >> diff --git a/drivers/clk/qcom/lcc-msm8660.c b/drivers/clk/qcom/lcc-msm8660.c
> > [..]
> >> +/* The vendor code uses PLL4 as parent everywhere */
> >> +static const struct parent_map lcc_parent_map[] = {
> >> +     { P_PXO, 0 },
> >> +     { P_CXO, 1 },
> >> +     /* Select RPM PLL4, but also used for selecting LPA PLL0 */
> >> +     { P_PLL4_LPA_PLL0, 2 },
> >> +     /* Will just ground the line */
> >> +     { P_GND, 6 },
> >> +};
> >> +
> >> +static const char * const lcc_parent_tbl[] = {
> >> +     "pxo",
> >> +     "cxo",
> >> +     /*
> >> +      * PLL4 is an RPM clock on MSM8660/APQ8060, set to "pll4" for this
> >> +      * If we enable and mux in the LPA_PLL0 on this platform, we can
> >> +      * set this to "lpa_pll0" instead
> >> +      */
> >> +     "pll4_clk",
> >> +     "gnd", /* This is a very inactive parent */
> >> +};
> >> +
> >> +/*
> >> + * This table is evidently for using PLL4 as parent, if we start using
> >> + * LPA_PLL0 we need to provide a second table.
> >> + */
> >
> > Aren't you muxing in LPA_PLL0 as source instead of PLL4 at the bottom of
> > probe()? And as you hard code that selector, shouldn't the parent table
> > reference lpa_pll0?
> 
> I think it's just a naming problem.
> 
> I'm actually using:
> 
> >> +     {   768000, P_PLL4_LPA_PLL0, 4,  1, 176 },
> 
> Because as far as I can tell, RPM clock PLL4 and LPA_PLL0 is the
> same thing, just different names.
> 
> I don't think the 8660 has its own LPA PLL, it's just the layers of
> confusion in the code that make it seem like so.
> 
> Or maybe someone was in the planning stages of adding the LPA PLL
> to the hardware and never got there. So some code confusingly
> refers to that...
> 
> I would really like to see how it actually works.
> 

8660 and 8960 are basically different revisions of the same SoC, so
reviewing clock-8x60.c and clock-8960.c found in below repository gives
some additional insight.

clock-8x60.c mentions LPA_PLL0 in _one_ comment and carries the
definitions for a LCC_PLL0 clock, but no references are made to these
defines. The code ever only references "pll4".

clock-8960.c goes beyond this and actually reference the LCC_PLL0
registers and they are all used to query and configure "pll4".


So AFAICT PLL4 is controlled/implemented in the LCC block and is called
PLL0 in that context and that on 8660 it's completely controlled by the
RPM.

As such I think it makes sense - on 8660 - to just reference "pll4",
perhaps with some comment in the code referencing LCC_PLL0 as the actual
implementation.

> BTW: do you have a pointer to a vendor tree for a Sony thing using
> the MSM8660 or APQ8060 that I can look at? I need more known
> working code to inspect.
> 

The tree I used for AOSP work on Xperia S can be found here:

https://github.com/sonyxperiadev/kernel/tree/nozomi/M8260AAABQNLZA30145

Regards,
Bjorn

Re: [PATCH 5/5 v2] clk: qcom: Add support for MSM8660 LCC

[Stephen Boyd]

On 05/30, Bjorn Andersson wrote:
> On Mon 29 May 05:23 PDT 2017, Linus Walleij wrote:
> 
> > On Sat, May 27, 2017 at 10:19 PM, Bjorn Andersson
> > <bjorn.andersson@linaro.org> wrote:
> > > On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:
> > >> diff --git a/drivers/clk/qcom/lcc-msm8660.c b/drivers/clk/qcom/lcc-msm8660.c
> > > [..]
> > >> +/* The vendor code uses PLL4 as parent everywhere */
> > >> +static const struct parent_map lcc_parent_map[] = {
> > >> +     { P_PXO, 0 },
> > >> +     { P_CXO, 1 },
> > >> +     /* Select RPM PLL4, but also used for selecting LPA PLL0 */
> > >> +     { P_PLL4_LPA_PLL0, 2 },
> > >> +     /* Will just ground the line */
> > >> +     { P_GND, 6 },
> > >> +};
> > >> +
> > >> +static const char * const lcc_parent_tbl[] = {
> > >> +     "pxo",
> > >> +     "cxo",
> > >> +     /*
> > >> +      * PLL4 is an RPM clock on MSM8660/APQ8060, set to "pll4" for this
> > >> +      * If we enable and mux in the LPA_PLL0 on this platform, we can
> > >> +      * set this to "lpa_pll0" instead
> > >> +      */
> > >> +     "pll4_clk",
> > >> +     "gnd", /* This is a very inactive parent */
> > >> +};
> > >> +
> > >> +/*
> > >> + * This table is evidently for using PLL4 as parent, if we start using
> > >> + * LPA_PLL0 we need to provide a second table.
> > >> + */
> > >
> > > Aren't you muxing in LPA_PLL0 as source instead of PLL4 at the bottom of
> > > probe()? And as you hard code that selector, shouldn't the parent table
> > > reference lpa_pll0?
> > 
> > I think it's just a naming problem.
> > 
> > I'm actually using:
> > 
> > >> +     {   768000, P_PLL4_LPA_PLL0, 4,  1, 176 },
> > 
> > Because as far as I can tell, RPM clock PLL4 and LPA_PLL0 is the
> > same thing, just different names.
> > 
> > I don't think the 8660 has its own LPA PLL, it's just the layers of
> > confusion in the code that make it seem like so.
> > 
> > Or maybe someone was in the planning stages of adding the LPA PLL
> > to the hardware and never got there. So some code confusingly
> > refers to that...
> > 
> > I would really like to see how it actually works.
> > 
> 
> 8660 and 8960 are basically different revisions of the same SoC, so
> reviewing clock-8x60.c and clock-8960.c found in below repository gives
> some additional insight.
> 
> clock-8x60.c mentions LPA_PLL0 in _one_ comment and carries the
> definitions for a LCC_PLL0 clock, but no references are made to these
> defines. The code ever only references "pll4".
> 
> clock-8960.c goes beyond this and actually reference the LCC_PLL0
> registers and they are all used to query and configure "pll4".
> 
> 
> So AFAICT PLL4 is controlled/implemented in the LCC block and is called
> PLL0 in that context and that on 8660 it's completely controlled by the
> RPM.
> 
> As such I think it makes sense - on 8660 - to just reference "pll4",
> perhaps with some comment in the code referencing LCC_PLL0 as the actual
> implementation.

Correct, LCC_PLL0 is a local naming scheme for the audio clock
controller's 0th PLL. That just so happens to be PLL4 "globally"
in the SoC. From global clock control standpoint it's PLL4. I
would just reference PLL4 everywhere and ignore the whole
LCC_PLL0 thing.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Stephen Boyd]

On 05/24, Linus Walleij wrote:
> On Wed, May 17, 2017 at 9:23 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Wed, Apr 19, 2017 at 11:13 AM, Linus Walleij
> > <linus.walleij@linaro.org> wrote:
> >
> >> These compatible strings need to be added to extend support
> >> for the RPM CC to cover MSM8660/APQ8060. We also need to add
> >> enumberators to the include file for a few clocks that were
> >> missing.
> >>
> >> Cc: devicetree@vger.kernel.org
> >> Acked-by: Rob Herring <robh@kernel.org>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> ---
> >> ChangeLog v1->v2:
> >> - Collect Rob's ACK
> >
> > Ping on this patch set. It's been sitting for a month and was
> > not changed from v1 either, if you want me to change something
> > it'd be good to know.
> 
> Ping on this again.
> 
> I am sorry if you guys are swamped, I just don't know what to do,
> clock drivers are starting to become the bottleneck for everything
> I do. I bet the ARM SoC maintainers must be noticing it too.
> 
> Unfortunately I am already managing two subsystems and cannot
> volunteer to help out with a third, I would blow a fuse.
> 
> But it seems the CLK subsystem could use some co-maintainers?
> 
> Peter de Schrijver is a big authority in all things clocking (I clearly
> remember him discussing the subject a lot at conferences with lots
> of good points) so I would ask him to help, but I don't know how busy
> Peter may be with nVidia business etc these days...

Meh. Life happens and msm8660 is about a decade old now and not
something that I can easily review without digging through old
websites to find documentation on the code I worked on 7 years
ago. So sorry, but these sorts of patches are basically
approaching priority none for me and I have to steal time to
review them.

I'm getting back up to speed though on my mails, so things should
be good. Let me know if things seem broken next week please.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 2/5 v2] clk: qcom: Elaborate on "active" clocks in the RPM clock bindings

[Stephen Boyd]

On 04/28, Rob Herring wrote:
> On Wed, Apr 19, 2017 at 11:13:23AM +0200, Linus Walleij wrote:
> > The concept of "active" clocks is just explained in a bried comment in the
> > device driver, let's explain it a bit more in the device tree bindings
> > so everyone understands this.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v1->v2:
> > - Reword slighty in accordance with Stephens feedback.
> > ---
> >  Documentation/devicetree/bindings/clock/qcom,rpmcc.txt | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> 
> Despite my objections, you're just documenting what is already there.
> 
> Acked-by: Rob Herring <robh@kernel.org>

Perhaps this can be put into the clk driver instead? Is that the
concern?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Stephen Boyd]

On 04/19, Linus Walleij wrote:
> These compatible strings need to be added to extend support
> for the RPM CC to cover MSM8660/APQ8060. We also need to add
> enumberators to the include file for a few clocks that were
> missing.
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---

Applied to clk-qcom-rpm

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Stephen Boyd]

On 04/19, Linus Walleij wrote:
> These compatible strings need to be added to extend support
> for the RPM CC to cover MSM8660/APQ8060. We also need to add
> enumberators to the include file for a few clocks that were
> missing.
> 
> Cc: devicetree@vger.kernel.org
> Acked-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Applied to clk-qcom-rpm

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/5 v2] clk: qcom: Implement RPM clocks for MSM8660/APQ8060

[Stephen Boyd]

On 04/19, Linus Walleij wrote:
> diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> index df3e5fe8442a..61c67e93bea3 100644
> --- a/drivers/clk/qcom/clk-rpm.c
> +++ b/drivers/clk/qcom/clk-rpm.c
> @@ -56,6 +56,30 @@
>  		},							      \
>  	}
>  
> +#define DEFINE_CLK_RPM_FIXED(_platform, _name, _active, r_id, r)	      \
> +	static struct clk_rpm _platform##_##_name = {			      \
> +		.rpm_clk_id = (r_id),					      \
> +		.rate = (r),						      \
> +		.hw.init = &(struct clk_init_data){			      \
> +			.ops = &clk_rpm_fixed_ops,			      \
> +			.name = #_name,					      \
> +			.parent_names = (const char *[]){ "pxo_board" },      \

Parent should be pxo, not pxo_board.

> +			.num_parents = 1,				      \
> +		},							      \
> +	};								      \
> +	static struct clk_rpm _platform##_##_active = {			      \
> +		.rpm_clk_id = (r_id),					      \
> +		.peer = &_platform##_##_name,				      \
> +		.active_only = true,					      \
> +		.rate = (r),						      \
> +		.hw.init = &(struct clk_init_data){			      \
> +			.ops = &clk_rpm_fixed_ops,			      \
> +			.name = #_active,				      \
> +			.parent_names = (const char *[]){ "pxo_board" },      \

ditto.

> +			.num_parents = 1,				      \
> +		},							      \
> +	}
> +
>  #define DEFINE_CLK_RPM_PXO_BRANCH(_platform, _name, _active, r_id, r)	      \
>  	static struct clk_rpm _platform##_##_active;			      \
>  	static struct clk_rpm _platform##_##_name = {			      \
> @@ -143,6 +167,13 @@ static int clk_rpm_handoff(struct clk_rpm *r)
>  	int ret;
>  	u32 value = INT_MAX;
>  
> +	/*
> +	 * The vendor tree simply reads the status for this
> +	 * RPM clock.
> +	 */
> +	if (r->rpm_clk_id == QCOM_RPM_PLL_4)
> +		return 0;
> +

I can't recall what that was all about. Handoff code came later
than when this PLL was supported, so perhaps we just missed
handoff here. Really it probably doesn't matter because sounds
aren't being played in the bootloader and/or the PLL isn't
enabled that early.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 3/5 v2] clk: qcom: Implement RPM clocks for MSM8660/APQ8060

[Stephen Boyd]

On 05/27, Bjorn Andersson wrote:
> On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:
> 
> > The RPM clocks were missing for MSM8660/APQ8060. For this to be
> > completed we need to add a special fixed rate RPM clock that is used
> > for the PLL4 on these SoCs. The rest of the clocks are pretty
> > similar to the other supported platforms.
> > 
> > The "active" clock pattern is mirrored in all the clocks. I guess
> > that the PLL4 that clocks the LPASS is actually never used as
> > "active only" since the low-power audio subsystem should be left
> > on when the system suspends, so it can be used as a stand-alone
> > MP3 player type of device.
> > 
> > As we do not have firmware for the LPASS we will probably only use
> > this clock when the system is up and running (not suspended) for now,
> > so that will be using the "active" clock.
> > 
> 
> Note that "active" vs "sleep" is not related to the Linux suspend state,
> but rather the CPU idle state; at the bottom of the CPU idle path the
> RPM will react and reconfigure resources to their sleep state (if one is
> configured) and then reconfigured based on the active state before
> returning from the idle.
> 
> The PLL4 seems to be enabled only on behalf of the booting LPASS Hexagon
> - which will cast its own vote once its booted - and as such we only
> configure the active state (meaning both states will have same
> configuration).  The result is that PLL4 will be on from prepare() to
> unprepare() regardless of what the application CPU does.
> 
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v1->v2:
> > - Add the small hunk to the clk_rpm_handoff() function that just
> >   skip over this for the fixed PLL4 clock. This accidentally
> >   ended up in another patch.
> > ---
> >  drivers/clk/qcom/clk-rpm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 106 insertions(+)
> > 
> > diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> > index df3e5fe8442a..61c67e93bea3 100644
> > --- a/drivers/clk/qcom/clk-rpm.c
> > +++ b/drivers/clk/qcom/clk-rpm.c
> > @@ -56,6 +56,30 @@
> >  		},							      \
> >  	}
> >  
> > +#define DEFINE_CLK_RPM_FIXED(_platform, _name, _active, r_id, r)	      \
> 
> Is there a reason why you don't use DEFINE_CLK_RPM_PXO_BRANCH() for
> PLL4?
> 
> Looking at the downstream code PLL4 is explicitly handled differently
> and is only exposing one state. So if you're seeing issues with reusing
> the PXO_BRANCH() I think you should slim this down further and only
> register a single clk_rpm from this.

True. Except we would need a slight variant on PXO_BRANCH,
because PXO_BRANCH would be better if it returned the parent rate
from recalc_rate(), by just not having a recalc rate op set. The
parent is pxo_board/cxo_board and that will have the correct rate
for the board we're running on. In the PLL4 case, we want to
return the magic frequency that we know the PLL is running at
because that was the only configuration supported. So we would
need a special recalc op for that one clk here. But otherwise yes
it should be able to use most of the other code.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 5/5 v2] clk: qcom: Add support for MSM8660 LCC

[Stephen Boyd]

On 04/19, Linus Walleij wrote:
> This adds support for the MSM8660 Low-power audio
> subsystem (LPASS) clock controller (LCC). This is nice when
> you want to have audio from the system.
> 
> We currently only support using the PLL4 (which is an RPM
> clock) as the parent, however the LPASS has its own PLL named
> LPA_PLL0 that we can experiment with enabling later on, so
> for this reason the code contains a few hints on how to enable
> the LPA_PLL0.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - No changes.
> ---
>  drivers/clk/qcom/Kconfig       |   9 +
>  drivers/clk/qcom/Makefile      |   1 +
>  drivers/clk/qcom/lcc-msm8660.c | 418 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 428 insertions(+)
>  create mode 100644 drivers/clk/qcom/lcc-msm8660.c
> 
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 5fb8d7430908..d657d94d372f 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -90,6 +90,15 @@ config MSM_GCC_8660
>  	  Say Y if you want to use peripheral devices such as UART, SPI,
>  	  i2c, USB, SD/eMMC, etc.
>  
> +config MSM_LCC_8660
> +	tristate "MSM8660 LPASS Clock Controller"
> +	select MSM_GCC_8660
> +	depends on COMMON_CLK_QCOM

depends on MSM_GCC_8660? Or perhaps the RPM driver. The intent is
to make sure the driver can't be built unless the dependency clks
that are inputs to the controller are also built. We may want to
just get rid of that sort of dependency, but that's the design
right now.

> +	help
> +	  Support for the Low-power Audio Subsystem (LPASS) clock controller
> +	  on MSM8660 devices.
> +	  Say Y if you want to use audio devices such as I2S and PCM.
> +
>  config MSM_GCC_8916
>  	tristate "MSM8916 Global Clock Controller"
>  	select QCOM_GDSC
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 1c3e222b917b..d6d5ab178522 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_IPQ_LCC_806X) += lcc-ipq806x.o
>  obj-$(CONFIG_MDM_GCC_9615) += gcc-mdm9615.o
>  obj-$(CONFIG_MDM_LCC_9615) += lcc-mdm9615.o
>  obj-$(CONFIG_MSM_GCC_8660) += gcc-msm8660.o
> +obj-$(CONFIG_MSM_LCC_8660) += lcc-msm8660.o
>  obj-$(CONFIG_MSM_GCC_8916) += gcc-msm8916.o
>  obj-$(CONFIG_MSM_GCC_8960) += gcc-msm8960.o
>  obj-$(CONFIG_MSM_GCC_8974) += gcc-msm8974.o
> diff --git a/drivers/clk/qcom/lcc-msm8660.c b/drivers/clk/qcom/lcc-msm8660.c
> new file mode 100644
> index 000000000000..73dc8ede6a2a
> --- /dev/null
> +++ b/drivers/clk/qcom/lcc-msm8660.c
> @@ -0,0 +1,418 @@
> +/*
> + * Qualcomm MSM8660/APQ8060 Low-power Audio Subsystem (LPASS) Clock Controller
> + * Copyright (c) 2017 Linus Walleij <linus.walleij@linaro.org>
> + *
> + * Based on a copy of the IPQ806x driver
> + * (C) 2014 Rajendra Nayak

I don't see that in the file. Perhaps you meant to copy the linux
foundation one like in mdm9615?

> + * and portions of the MDM9615 driver
> + * (C) 2014 Neil Armstrong.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/bitops.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>

Is this used?

> +#include <linux/clk-provider.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,lcc-msm8660.h>
> +
> +#include "common.h"
> +#include "clk-regmap.h"
> +#include "clk-pll.h"
> +#include "clk-rcg.h"
> +#include "clk-branch.h"
> +#include "clk-regmap-divider.h"
> +#include "clk-regmap-mux.h"
> +
> +/*
> + * The vendor tree calls this "PLL0" but we are going to refer to it as
> + * LPA_PLL0 so as not to confuse it with the PLL0 on the GCC.
> + */
> +static struct clk_pll lpa_pll0 = {
> +	.l_reg = 0x4,
> +	.m_reg = 0x8,
> +	.n_reg = 0xc,
> +	.config_reg = 0x14,
> +	.mode_reg = 0x0,
> +	.status_reg = 0x18,
> +	.status_bit = 16,
> +	.clkr.hw.init = &(struct clk_init_data){
> +		.name = "lpa_pll0",

Just PLL4 please.

> +		.parent_names = (const char *[]){ "pxo" },
> +		.num_parents = 1,
> +		.ops = &clk_pll_ops,
> +	},
> +};
> +
> +/*
> + * The l/m/n values were read out of the registers after a cold boot.
> + * The config register read 00c22080 and the mode register 0x00000007.

0x00c22080?

> + */
> +static const struct pll_config lpa_pll0_config = {
> +	.l = 0xf,
> +	.m = 0x1c,
> +	.n = 0x465,
> +	.vco_val = BIT(17),
> +	.vco_mask = BIT(17) | BIT(16),
> +	.pre_div_val = 0x0,
> +	.pre_div_mask = BIT(19),
> +	.post_div_val = 0x0,
> +	.post_div_mask = BIT(21) | BIT(20),
> +	.mn_ena_mask = BIT(22),
> +	.main_output_mask = BIT(23),
> +};
> +
> +enum {
> +	P_PXO,
> +	P_CXO,
> +	P_PLL4_LPA_PLL0,

Just call it P_PLL4 please.

> +	P_GND,

Please remove ground.

> +};
> +
> +/* The vendor code uses PLL4 as parent everywhere */
> +static const struct parent_map lcc_parent_map[] = {
> +	{ P_PXO, 0 },
> +	{ P_CXO, 1 },
> +	/* Select RPM PLL4, but also used for selecting LPA PLL0 */
> +	{ P_PLL4_LPA_PLL0, 2 },
> +	/* Will just ground the line */
> +	{ P_GND, 6 },

We don't do clk grounding upstream so we don't need this. We can
add grounding in the future if anyone cares.

> +};
> +
> +static const char * const lcc_parent_tbl[] = {
> +	"pxo",
> +	"cxo",
> +	/*
> +	 * PLL4 is an RPM clock on MSM8660/APQ8060, set to "pll4" for this
> +	 * If we enable and mux in the LPA_PLL0 on this platform, we can
> +	 * set this to "lpa_pll0" instead

This comment is confusing because PLL4 is LPA_PLL0.

> +	 */
> +	"pll4_clk",

And just pll4. We don't need _clk for that. If that affects RPM
driver, it should be fixed as well.

> +	"gnd", /* This is a very inactive parent */

Heh. Is it needed because an audio clk is set to ground by
default?  That's the only case where I think it may be needed,
but I don't think we have that case where we need to change the
parent from ground to something else. If so, then we'll need to
add it as a valid source and then register it in this driver as a
fixed rate clk of rate 0.

> +};
> +
> +/*
> + * This table is evidently for using PLL4 as parent, if we start using
> + * LPA_PLL0 we need to provide a second table.
> + */

Remove comment.

> +static struct freq_tbl clk_tbl_aif_osr_pll4[] = {
> +	{   768000, P_PLL4_LPA_PLL0, 4,  1, 176 },
> +	{  1024000, P_PLL4_LPA_PLL0, 4,  1, 132 },
> +	{  1536000, P_PLL4_LPA_PLL0, 4,  1,  88 },
> +	{  2048000, P_PLL4_LPA_PLL0, 4,  1,  66 },
> +	{  3072000, P_PLL4_LPA_PLL0, 4,  1,  44 },
> +	{  4096000, P_PLL4_LPA_PLL0, 4,  1,  33 },
> +	{  6144000, P_PLL4_LPA_PLL0, 4,  1,  22 },
> +	{  8192000, P_PLL4_LPA_PLL0, 2,  1,  33 },
> +	{ 12288000, P_PLL4_LPA_PLL0, 4,  1,  11 },
> +	{ 24576000, P_PLL4_LPA_PLL0, 2,  1,  11 },
> +	{ 27000000, P_PXO,           1,  0,   0 },
> +	{ }
> +};
> +
> +/*
> + * This macro is modified from lcc-mdm9516.c, it's used for all the
> + * AIF clocks, all of them have an OSR clock and a bit clock derived
> + * from the OSR clock.
> + *
> + * These clocks differ from many other platforms by using
> + * BRANCH_HALT_DELAY for the *_bit_div_clk
> + */

Ok. Please leave out this comment.

> +#define CLK_AIF_OSR_DIV(prefix, _ns, _md, _hr)			\
> +static struct clk_rcg prefix##_osr_src = {			\
> +	.ns_reg = _ns,						\
> +	.md_reg = _md,						\
> +	.mn = {							\
> +		.mnctr_en_bit = 8,				\
> +		.mnctr_reset_bit = 7,				\
> +		.mnctr_mode_shift = 5,				\
> +		.n_val_shift = 24,				\
> +		.m_val_shift = 8,				\
> +		.width = 8,					\
> +	},							\
> +	.p = {							\
> +		.pre_div_shift = 3,				\
> +		.pre_div_width = 2,				\
> +	},							\
> +	.s = {							\
> +		.src_sel_shift = 0,				\
> +		.parent_map = lcc_parent_map,			\
> +	},							\
> +	.freq_tbl = clk_tbl_aif_osr_pll4,			\
> +	.clkr = {						\
> +		.enable_reg = _ns,				\
> +		.enable_mask = BIT(9),				\
> +		.hw.init = &(struct clk_init_data){		\
> +			.name = #prefix "_osr_src",		\
> +			.parent_names = lcc_parent_tbl,		\
> +			.num_parents = 4,			\
> +			.ops = &clk_rcg_ops,			\
> +			.flags = CLK_SET_RATE_GATE,		\
> +		},						\
> +	},							\
> +};								\
> +								\
> +static const char * const lcc_##prefix##_parents[] = {		\
> +	#prefix "_osr_src",					\
> +};								\
> +								\
> +static struct clk_branch prefix##_osr_clk = {			\
> +	.halt_reg = _hr,					\
> +	.halt_bit = 1,						\
> +	.halt_check = BRANCH_HALT_ENABLE,			\
> +	.clkr = {						\
> +		.enable_reg = _ns,				\
> +		.enable_mask = BIT(17),				\
> +		.hw.init = &(struct clk_init_data){		\
> +			.name = #prefix "_osr_clk",		\
> +			.parent_names = lcc_##prefix##_parents,	\
> +			.num_parents = 1,			\
> +			.ops = &clk_branch_ops,			\
> +			.flags = CLK_SET_RATE_PARENT,		\
> +		},						\
> +	},							\
> +};								\
> +								\
> +static struct clk_regmap_div prefix##_div_clk = {		\
> +	.reg = _ns,						\
> +	.shift = 10,						\
> +	.width = 4,						\
> +	.clkr = {						\
> +		.hw.init = &(struct clk_init_data){		\
> +			.name = #prefix "_div_clk",		\
> +			.parent_names = lcc_##prefix##_parents,	\
> +			.num_parents = 1,			\
> +			.ops = &clk_regmap_div_ops,		\
> +		},						\
> +	},							\
> +};								\
> +								\
> +static struct clk_branch prefix##_bit_div_clk = {		\
> +	.halt_reg = _hr,					\
> +	.halt_bit = 0,						\
> +	.halt_check = BRANCH_HALT_DELAY,			\
> +	.clkr = {						\
> +		.enable_reg = _ns,				\
> +		.enable_mask = BIT(15),				\
> +		.hw.init = &(struct clk_init_data){		\
> +			.name = #prefix "_bit_div_clk",		\
> +			.parent_names = (const char *[]){	\
> +				#prefix "_div_clk"		\
> +			},					\
> +			.num_parents = 1,			\
> +			.ops = &clk_branch_ops,			\
> +			.flags = CLK_SET_RATE_PARENT,		\
> +		},						\
> +	},							\
> +};								\
> +								\
> +static struct clk_regmap_mux prefix##_bit_clk = {		\
> +	.reg = _ns,						\
> +	.shift = 14,						\
> +	.width = 1,						\
> +	.clkr = {						\
> +		.hw.init = &(struct clk_init_data){		\
> +			.name = #prefix "_bit_clk",		\
> +			.parent_names = (const char *[]){	\
> +				#prefix "_bit_div_clk",		\
> +				#prefix "_codec_clk",		\
> +			},					\
> +			.num_parents = 2,			\
> +			.ops = &clk_regmap_mux_closest_ops,	\
> +			.flags = CLK_SET_RATE_PARENT,		\
> +		},						\
> +	},							\
> +}
> +
> +CLK_AIF_OSR_DIV(mi2s, 0x48, 0x4c, 0x50);
> +CLK_AIF_OSR_DIV(codec_i2s_mic, 0x60, 0x64, 0x68);
> +CLK_AIF_OSR_DIV(spare_i2s_mic, 0x78, 0x7c, 0x80);
> +CLK_AIF_OSR_DIV(codec_i2s_spkr, 0x6c, 0x70, 0x74);
> +CLK_AIF_OSR_DIV(spare_i2s_spkr, 0x84, 0x88, 0x8c);
> +
> +/*
> + * PCM clock
> + * This table is evidently for using PLL4 as parent, if we start using
> + * LPA_PLL0 we need to provide a second table.
> + */

Remove comment.

> +static struct freq_tbl clk_tbl_pcm_pll4[] = {
> +	{   512000, P_PLL4_LPA_PLL0, 4, 1, 264 },
> +	{   768000, P_PLL4_LPA_PLL0, 4, 1, 176 },
> +	{  1024000, P_PLL4_LPA_PLL0, 4, 1, 132 },
> +	{  1536000, P_PLL4_LPA_PLL0, 4, 1,  88 },
> +	{  2048000, P_PLL4_LPA_PLL0, 4, 1,  66 },
> +	{  3072000, P_PLL4_LPA_PLL0, 4, 1,  44 },
> +	{  4096000, P_PLL4_LPA_PLL0, 4, 1,  33 },
> +	{  6144000, P_PLL4_LPA_PLL0, 4, 1,  22 },
> +	{  8192000, P_PLL4_LPA_PLL0, 2, 1,  33 },
> +	{ 12288000, P_PLL4_LPA_PLL0, 4, 1,  11 },
> +	{ 24580000, P_PLL4_LPA_PLL0, 2, 1,  11 },
> +	{ 27000000, P_PXO,           1, 0,   0 },
> +	{ },
> +};
[...]
> +
> +static int lcc_msm8660_probe(struct platform_device *pdev)
> +{
> +	u32 val;
> +	struct regmap *regmap;
> +
> +	regmap = qcom_cc_map(pdev, &lcc_msm8660_desc);
> +	if (IS_ERR(regmap))
> +		return PTR_ERR(regmap);
> +
> +	/* Configure the rate of LPA_PLL0 if the bootloader hasn't already */
> +	regmap_read(regmap, 0x0, &val);
> +	if (!val) {
> +		dev_info(&pdev->dev, "configuring LPA_PLL0\n");
> +		clk_pll_configure_sr(&lpa_pll0, regmap, &lpa_pll0_config, true);
> +	} else {
> +		dev_info(&pdev->dev,
> +			 "LPA_PLL0 already configured\n");

dev_dbg? Is this the case that happens for you? I don't think we
should be programming the hardware because the RPM controls the
PLL. It should already be setup to the right rate. I don't see
this happening in the downstream code either, so this should all
be removed most likely.

> +	}
> +
> +	/*
> +	 * Enable LPA_PLL0 source on the LPASS Primary PLL Mux. Incidentally
> +	 * this is set to 0x00000001 at boot.
> +	 * 0x01 = LPA_PLL0
> +	 */
> +	regmap_write(regmap, 0xc4, 0x1);

And this isn't needed either?

> +
> +	return qcom_cc_really_probe(pdev, &lcc_msm8660_desc, regmap);
> +}
> +

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/5 v2] clk: qcom: Update DT bindings for the MSM8660/APQ8060 RPMCC

[Linus Walleij]

On Thu, Jun 1, 2017 at 9:38 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/24, Linus Walleij wrote:

>> I am sorry if you guys are swamped, I just don't know what to do,
>> clock drivers are starting to become the bottleneck for everything
>> I do. I bet the ARM SoC maintainers must be noticing it too.
(...)
> Meh. Life happens and msm8660 is about a decade old now and not
> something that I can easily review without digging through old
> websites to find documentation on the code I worked on 7 years
> ago. So sorry, but these sorts of patches are basically
> approaching priority none for me and I have to steal time to
> review them.

Sorry the reference above was mainly to the Gemini SoC clock driver
which was blocking ARM consolidation, the fact that this MSM8660
thing was stalled too just got me to think it was related.

I understand the MSM8660 is low prio, but the first DragonBoard
was pushed to developers en masse in 2012 (yours truly included),
which was just half a decade ago, and started
the community boards effort from Qualcomm, and yeah, community
moves slower than markets usually, the board is still supported by
the BSquare company that managed the board release for Qualcomm,
I can still login and check schematics and support forums,
cool isn't it :D

Yours,
Linus Walleij

Re: [PATCH 3/5 v2] clk: qcom: Implement RPM clocks for MSM8660/APQ8060

[Linus Walleij]

On Sat, May 27, 2017 at 10:00 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Wed 19 Apr 02:13 PDT 2017, Linus Walleij wrote:

>> The "active" clock pattern is mirrored in all the clocks. I guess
>> that the PLL4 that clocks the LPASS is actually never used as
>> "active only" since the low-power audio subsystem should be left
>> on when the system suspends, so it can be used as a stand-alone
>> MP3 player type of device.
>>
>> As we do not have firmware for the LPASS we will probably only use
>> this clock when the system is up and running (not suspended) for now,
>> so that will be using the "active" clock.
>>
>
> Note that "active" vs "sleep" is not related to the Linux suspend state,
> but rather the CPU idle state; at the bottom of the CPU idle path the
> RPM will react and reconfigure resources to their sleep state (if one is
> configured) and then reconfigured based on the active state before
> returning from the idle.
>
> The PLL4 seems to be enabled only on behalf of the booting LPASS Hexagon
> - which will cast its own vote once its booted - and as such we only
> configure the active state (meaning both states will have same
> configuration).  The result is that PLL4 will be on from prepare() to
> unprepare() regardless of what the application CPU does.

OK I copy/pasted some of this into my commit message and cut
the "active" clock from PLL4.

>> +#define DEFINE_CLK_RPM_FIXED(_platform, _name, _active, r_id, r)           \
>
> Is there a reason why you don't use DEFINE_CLK_RPM_PXO_BRANCH() for
> PLL4?

So it is (I think as concluded from the other mail) a pretty hardwired
PLL that can only be turned on and off.

#define DEFINE_CLK_RPM_PXO_BRANCH(_platform, _name, _active, r_id, r)

This macro presupposes an _active variant, so it's not really
working :/

Nothing in the driver is using this macro however, it seems like
it was defined for some missing piece.

How do you feel about if I simply update that macro and
cut the _active version then?

Or should I create a new DEFINE_CLK_RPM_PXO_BRANCH_FIXED()?

Yours,
Linus Walleij