[PATCH v2 0/4] Prepare general purpose clocks on msm8916

[PATCH v2 0/4] Prepare general purpose clocks on msm8916

[Nikita Travkin]

Some devices make use of general purpose clocks as PWM outputs by
controlling their duty cycle.

Notably, many devices (e.g. Samsung A3/A5, LG G Watch R and probably
many others) use clock based PWM to control the haptic feedback,
some other can control backlight or flash/torch LED brightness.

As a follow-up to a proposed clock based PWM output driver [1],
this series contains various fixes to make it useful on msm8916
based devices.

[1] - https://lore.kernel.org/lkml/20220612132203.290726-1-nikita@trvn.ru/T/#t

Changes since v1:
 - Use clamp() instead of two boundary checks

Nikita Travkin (4):
  clk: qcom: clk-rcg2: Fail Duty-Cycle configuration if MND divider is
    not enabled.
  clk: qcom: clk-rcg2: Make sure to not write d=0 to the NMD register
  pinctrl: qcom: msm8916: Allow CAMSS GP clocks to be muxed
  clk: qcom: gcc-msm8916: Add rates to the GP clocks

 drivers/clk/qcom/clk-rcg2.c            | 16 +++++++++---
 drivers/clk/qcom/gcc-msm8916.c         | 35 ++++++++++++++++++++++++++
 drivers/pinctrl/qcom/pinctrl-msm8916.c |  4 +--
 3 files changed, 49 insertions(+), 6 deletions(-)

-- 
2.35.3

[PATCH v2 1/4] clk: qcom: clk-rcg2: Fail Duty-Cycle configuration if MND divider is not enabled.

[Nikita Travkin]

In cases when MND is not enabled (e.g. when only Half Integer Divider is
used), setting D registers makes no effect.

Fail instead of making ineffective write.

Fixes: 7f891faf596e ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/clk/qcom/clk-rcg2.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 8e5dce09d162..2375e8122012 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -437,7 +437,7 @@ static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
 static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
 {
 	struct clk_rcg2 *rcg = to_clk_rcg2(hw);
-	u32 notn_m, n, m, d, not2d, mask, duty_per;
+	u32 notn_m, n, m, d, not2d, mask, duty_per, cfg;
 	int ret;
 
 	/* Duty-cycle cannot be modified for non-MND RCGs */
@@ -448,6 +448,11 @@ static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
 
 	regmap_read(rcg->clkr.regmap, RCG_N_OFFSET(rcg), &notn_m);
 	regmap_read(rcg->clkr.regmap, RCG_M_OFFSET(rcg), &m);
+	regmap_read(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), &cfg);
+
+	/* Duty-cycle cannot be modified if MND divider is in bypass mode. */
+	if (!(cfg & CFG_MODE_MASK))
+		return -EINVAL;
 
 	n = (~(notn_m) + m) & mask;
 
-- 
2.35.3

Re: [PATCH v2 1/4] clk: qcom: clk-rcg2: Fail Duty-Cycle configuration if MND divider is not enabled.

[Stephen Boyd]

Quoting Nikita Travkin (2022-06-12 07:59:52)
> In cases when MND is not enabled (e.g. when only Half Integer Divider is
> used), setting D registers makes no effect.
> 
> Fail instead of making ineffective write.
> 
> Fixes: 7f891faf596e ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

[PATCH v2 2/4] clk: qcom: clk-rcg2: Make sure to not write d=0 to the NMD register

[Nikita Travkin]

Sometimes calculation of d value may result in 0 because of the
rounding after integer division. This causes the following error:

[  113.969689] camss_gp1_clk_src: rcg didn't update its configuration.
[  113.969754] WARNING: CPU: 3 PID: 35 at drivers/clk/qcom/clk-rcg2.c:122 update_config+0xc8/0xdc

Make sure that D value is never zero.

Fixes: 7f891faf596e ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>

---
v2:
 - Use clamp()
---
 drivers/clk/qcom/clk-rcg2.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index 2375e8122012..28019edd2a50 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -13,6 +13,7 @@
 #include <linux/rational.h>
 #include <linux/regmap.h>
 #include <linux/math64.h>
+#include <linux/minmax.h>
 #include <linux/slab.h>
 
 #include <asm/div64.h>
@@ -461,9 +462,11 @@ static int clk_rcg2_set_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
 	/* Calculate 2d value */
 	d = DIV_ROUND_CLOSEST(n * duty_per * 2, 100);
 
-	 /* Check bit widths of 2d. If D is too big reduce duty cycle. */
-	if (d > mask)
-		d = mask;
+	/*
+	 * Check bit widths of 2d. If D is too big reduce duty cycle.
+	 * Also make sure it is never zero.
+	 */
+	d = clamp_val(d, 1, mask);
 
 	if ((d / 2) > (n - m))
 		d = (n - m) * 2;
-- 
2.35.3

Re: [PATCH v2 2/4] clk: qcom: clk-rcg2: Make sure to not write d=0 to the NMD register

[Stephen Boyd]

Quoting Nikita Travkin (2022-06-12 07:59:53)
> Sometimes calculation of d value may result in 0 because of the
> rounding after integer division. This causes the following error:
> 
> [  113.969689] camss_gp1_clk_src: rcg didn't update its configuration.
> [  113.969754] WARNING: CPU: 3 PID: 35 at drivers/clk/qcom/clk-rcg2.c:122 update_config+0xc8/0xdc
> 
> Make sure that D value is never zero.
> 
> Fixes: 7f891faf596e ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> 
> ---

Reviewed-by: Stephen Boyd <sboyd@kernel.org>

[PATCH v2 3/4] pinctrl: qcom: msm8916: Allow CAMSS GP clocks to be muxed

[Nikita Travkin]

GPIO 31, 32 can be muxed to GCC_CAMSS_GP(1,2)_CLK respectively but the
function was never assigned to the pingroup (even though the function
exists already).

Add this mode to the related pins.

Fixes: 5373a2c5abb6 ("pinctrl: qcom: Add msm8916 pinctrl driver")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/pinctrl/qcom/pinctrl-msm8916.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-msm8916.c b/drivers/pinctrl/qcom/pinctrl-msm8916.c
index 396db12ae904..bf68913ba821 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm8916.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm8916.c
@@ -844,8 +844,8 @@ static const struct msm_pingroup msm8916_groups[] = {
 	PINGROUP(28, pwr_modem_enabled_a, NA, NA, NA, NA, NA, qdss_tracedata_b, NA, atest_combodac),
 	PINGROUP(29, cci_i2c, NA, NA, NA, NA, NA, qdss_tracedata_b, NA, atest_combodac),
 	PINGROUP(30, cci_i2c, NA, NA, NA, NA, NA, NA, NA, qdss_tracedata_b),
-	PINGROUP(31, cci_timer0, NA, NA, NA, NA, NA, NA, NA, NA),
-	PINGROUP(32, cci_timer1, NA, NA, NA, NA, NA, NA, NA, NA),
+	PINGROUP(31, cci_timer0, flash_strobe, NA, NA, NA, NA, NA, NA, NA),
+	PINGROUP(32, cci_timer1, flash_strobe, NA, NA, NA, NA, NA, NA, NA),
 	PINGROUP(33, cci_async, NA, NA, NA, NA, NA, NA, NA, qdss_tracedata_b),
 	PINGROUP(34, pwr_nav_enabled_a, NA, NA, NA, NA, NA, NA, NA, qdss_tracedata_b),
 	PINGROUP(35, pwr_crypto_enabled_a, NA, NA, NA, NA, NA, NA, NA, qdss_tracedata_b),
-- 
2.35.3

Re: [PATCH v2 3/4] pinctrl: qcom: msm8916: Allow CAMSS GP clocks to be muxed

[Linus Walleij]

On Sun, Jun 12, 2022 at 5:00 PM Nikita Travkin <nikita@trvn.ru> wrote:

> GPIO 31, 32 can be muxed to GCC_CAMSS_GP(1,2)_CLK respectively but the
> function was never assigned to the pingroup (even though the function
> exists already).
>
> Add this mode to the related pins.
>
> Fixes: 5373a2c5abb6 ("pinctrl: qcom: Add msm8916 pinctrl driver")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>

This patch 3/4 applied to the pinctrl tree so you have one less thing
to iterate (and one less person to involve).

Yours,
Linus Walleij

[PATCH v2 4/4] clk: qcom: gcc-msm8916: Add rates to the GP clocks

[Nikita Travkin]

msm8916 has (at least) 6 "General Purpose" clocks that can be muxed to
SoC pins. These clocks are:

GP_CLK{0, 1} : GPIO_{31, 32} (Belongs to CAMSS according to Linux)
GP_CLK_{1-3}{A, B} : GPIO_{49-51, 97, 12, 13} (Belongs to GCC itself)
GP_MN : GPIO_110 (Doesn't seem to be described in gcc,
    ignored in this patch)

Those clocks may be used as e.g. PWM sources for external peripherals.
Add more frequencies to the table for those clocks so it's possible
for arbitrary peripherals to make use of them.

Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/clk/qcom/gcc-msm8916.c | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/clk/qcom/gcc-msm8916.c b/drivers/clk/qcom/gcc-msm8916.c
index 17e4a5a2a9fd..9a46794f6eb8 100644
--- a/drivers/clk/qcom/gcc-msm8916.c
+++ b/drivers/clk/qcom/gcc-msm8916.c
@@ -765,7 +765,20 @@ static struct clk_rcg2 cci_clk_src = {
 	},
 };
 
+/*
+ * This is a frequency table for "General Purpose" clocks.
+ * These clocks can be muxed to the SoC pins and may be used by
+ * external devices. They're often used as PWM source.
+ *
+ * See comment at ftbl_gcc_gp1_3_clk.
+ */
 static const struct freq_tbl ftbl_gcc_camss_gp0_1_clk[] = {
+	F(10000,   P_XO,    16,  1, 120),
+	F(100000,  P_XO,    16,  1,  12),
+	F(500000,  P_GPLL0, 16,  1, 100),
+	F(1000000, P_GPLL0, 16,  1,  50),
+	F(2500000, P_GPLL0, 16,  1,  20),
+	F(5000000, P_GPLL0, 16,  1,  10),
 	F(100000000, P_GPLL0, 8, 0, 0),
 	F(200000000, P_GPLL0, 4, 0, 0),
 	{ }
@@ -927,7 +940,29 @@ static struct clk_rcg2 crypto_clk_src = {
 	},
 };
 
+/*
+ * This is a frequency table for "General Purpose" clocks.
+ * These clocks can be muxed to the SoC pins and may be used by
+ * external devices. They're often used as PWM source.
+ *
+ * Please note that MND divider must be enabled for duty-cycle
+ * control to be possible. (M != N) Also since D register is configured
+ * with a value multiplied by 2, and duty cycle is calculated as
+ *                             (2 * D) % 2^W
+ *                DutyCycle = ----------------
+ *                              2 * (N % 2^W)
+ * (where W = .mnd_width)
+ * N must be half or less than maximum value for the register.
+ * Otherwise duty-cycle control would be limited.
+ * (e.g. for 8-bit NMD N should be less than 128)
+ */
 static const struct freq_tbl ftbl_gcc_gp1_3_clk[] = {
+	F(10000,   P_XO,    16,  1, 120),
+	F(100000,  P_XO,    16,  1,  12),
+	F(500000,  P_GPLL0, 16,  1, 100),
+	F(1000000, P_GPLL0, 16,  1,  50),
+	F(2500000, P_GPLL0, 16,  1,  20),
+	F(5000000, P_GPLL0, 16,  1,  10),
 	F(19200000, P_XO, 1, 0,	0),
 	{ }
 };
-- 
2.35.3

Re: (subset) [PATCH v2 0/4] Prepare general purpose clocks on msm8916

[Bjorn Andersson]

On Sun, 12 Jun 2022 19:59:51 +0500, Nikita Travkin wrote:
> Some devices make use of general purpose clocks as PWM outputs by
> controlling their duty cycle.
> 
> Notably, many devices (e.g. Samsung A3/A5, LG G Watch R and probably
> many others) use clock based PWM to control the haptic feedback,
> some other can control backlight or flash/torch LED brightness.
> 
> [...]

Applied, thanks!

[1/4] clk: qcom: clk-rcg2: Fail Duty-Cycle configuration if MND divider is not enabled.
      commit: bdafb609c3bb848d710ad9cd4debd2ee9d6a4049
[2/4] clk: qcom: clk-rcg2: Make sure to not write d=0 to the NMD register
      commit: d0696770cef35a1fd16ea2167e2198c18aa6fbfe
[4/4] clk: qcom: gcc-msm8916: Add rates to the GP clocks
      commit: bf8bb8eaccf4e68d79743da631f61252753ca7cd

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>