[PATCH v2 0/2] Introduce support for AM62 DSS VP0 Divider Clock

[PATCH v2 0/2] Introduce support for AM62 DSS VP0 Divider Clock

[Aradhya Bhatia]

Introduce clock divider driver and bindings for AM62 DSS (VP0).

The OLDI transmitters in AM62 SoC, require a serial clock, which is 7
times the pixel clock required by the Video Port 0 (VP0) of the DSS.

The clock architecture is such that the relevant PLL is connected to the
DSS VP0 through a clock divider (by-7). The DSS requests the pixel clock
to the clock divider, which forwards the request to parent PLL. The PLL,
in turn, is supposed to generate a 7x pixel-clock (serial clock) to feed
the OLDI transmitters directly, and the DSS through the said divider.
This ensures that both the OLDI TXes and DSS get their required clocks.

Change Log:
v2:
  - Create separate devicetree binding and driver for the clock.
    The previous version of the series added the compatible inside the
    fixed-factor clock driver, and explicitly set the flag
    CLK_SET_RATE_PARENT to have the set clock rate request propagate to
    the parent PLL. The driver is referenced from the fixed-factor clock
    driver and modified specifically to support AM62 DSS requirements.

Links:
V1: https://lore.kernel.org/all/20221226095745.19757-1-a-bhatia1@ti.com/

Aradhya Bhatia (2):
  dt-bindings: clock: Add binding documentation for TI AM62 DSS Clock
  clk: keystone: Add support AM62 DSS clock divider

 .../clock/ti,am62-dss-vp0-div-clk.yaml        |  44 +++++
 drivers/clk/keystone/Kconfig                  |   9 +
 drivers/clk/keystone/Makefile                 |   1 +
 drivers/clk/keystone/clk-am62-dss.c           | 164 ++++++++++++++++++
 4 files changed, 218 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml
 create mode 100644 drivers/clk/keystone/clk-am62-dss.c

-- 
2.39.1

[PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI AM62 DSS Clock

[Aradhya Bhatia]

Add DT bindings for DSS clock divider of TI's AM62 family of SoCs.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---
 .../clock/ti,am62-dss-vp0-div-clk.yaml        | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml

diff --git a/Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml b/Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml
new file mode 100644
index 000000000000..310d2a989d5b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/ti,am62-dss-vp0-div-clk.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI AM62 DSS - OLDI Divider Clock
+
+maintainers:
+  - Aradhya Bhatia <a-bhatia1@ti.com>
+
+properties:
+  compatible:
+    items:
+      - const: ti,am62-dss-vp0-div-clk
+
+  "#clock-cells":
+    const: 0
+
+  clocks:
+    maxItems: 1
+
+  clock-div:
+    description: Fixed divider
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+
+required:
+  - compatible
+  - clocks
+  - "#clock-cells"
+  - clock-div
+
+additionalProperties: false
+
+examples:
+  - |
+    clock {
+      compatible = "ti,am62-dss-vp0-div-clk";
+      clocks = <&parent_clock>;
+      #clock-cells = <0>;
+      clock-div = <7>;
+    };
+...
-- 
2.39.1

Re: [PATCH v2 1/2] dt-bindings: clock: Add binding documentation for TI AM62 DSS Clock

[Rob Herring]

On Mon, Feb 13, 2023 at 05:29:53PM +0530, Aradhya Bhatia wrote:
> Add DT bindings for DSS clock divider of TI's AM62 family of SoCs.

Subject: Drop 'binding documentation for '

> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
> ---
>  .../clock/ti,am62-dss-vp0-div-clk.yaml        | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml b/Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml
> new file mode 100644
> index 000000000000..310d2a989d5b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/ti,am62-dss-vp0-div-clk.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/ti,am62-dss-vp0-div-clk.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI AM62 DSS - OLDI Divider Clock
> +
> +maintainers:
> +  - Aradhya Bhatia <a-bhatia1@ti.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: ti,am62-dss-vp0-div-clk

What's wrong with 'fixed-factor-clock' binding?

> +
> +  "#clock-cells":
> +    const: 0
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-div:
> +    description: Fixed divider
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +
> +required:
> +  - compatible
> +  - clocks
> +  - "#clock-cells"
> +  - clock-div
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clock {
> +      compatible = "ti,am62-dss-vp0-div-clk";
> +      clocks = <&parent_clock>;
> +      #clock-cells = <0>;
> +      clock-div = <7>;
> +    };
> +...
> -- 
> 2.39.1
> 

[PATCH v2 2/2] clk: keystone: Add support AM62 DSS clock divider

[Aradhya Bhatia]

On TI's AM62 Family of SoCs, the pixel frequency of the DSS Video Port 0
comes from a by-7 clock divider. This is done to support the clock for
OLDI transmitters that need serial clock 7 times the pixel frequency.

A clock set request on this clock, is forwarded to its parent clock by
default, using the SET_RATE_PARENT flag. The PLL (in this case the
parent) generates serial clock for OLDI, which is then also fed to DSS
using this clock divider.

Signed-off-by: Aradhya Bhatia <a-bhatia1@ti.com>
---
 drivers/clk/keystone/Kconfig        |   9 ++
 drivers/clk/keystone/Makefile       |   1 +
 drivers/clk/keystone/clk-am62-dss.c | 164 ++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+)
 create mode 100644 drivers/clk/keystone/clk-am62-dss.c

diff --git a/drivers/clk/keystone/Kconfig b/drivers/clk/keystone/Kconfig
index e64d6726048f..543314ddfd2c 100644
--- a/drivers/clk/keystone/Kconfig
+++ b/drivers/clk/keystone/Kconfig
@@ -34,3 +34,12 @@ config TI_SYSCON_CLK
 	help
 	  This adds clock driver support for syscon based gate
 	  clocks on TI's K2 and K3 SoCs.
+
+config TI_AM62_DSS_CLK
+	tristate "Clock Divider for DSS VP0 of AM62 Family of SoCs"
+	depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
+	default TI_SCI_CLK
+	help
+	  This adds clock divider driver support for Video Port 0 of Display
+	  SubSystems (DSS) under AM62 Family of SoCs. This clock divider
+	  forwards a seventh (1/7) of the incoming clock.
diff --git a/drivers/clk/keystone/Makefile b/drivers/clk/keystone/Makefile
index 0e426e648f7c..3a683d622a60 100644
--- a/drivers/clk/keystone/Makefile
+++ b/drivers/clk/keystone/Makefile
@@ -2,3 +2,4 @@
 obj-$(CONFIG_COMMON_CLK_KEYSTONE)	+= pll.o gate.o
 obj-$(CONFIG_TI_SCI_CLK)		+= sci-clk.o
 obj-$(CONFIG_TI_SYSCON_CLK)		+= syscon-clk.o
+obj-$(CONFIG_TI_AM62_DSS_CLK)		+= clk-am62-dss.o
diff --git a/drivers/clk/keystone/clk-am62-dss.c b/drivers/clk/keystone/clk-am62-dss.c
new file mode 100644
index 000000000000..2c9fc4bc89e3
--- /dev/null
+++ b/drivers/clk/keystone/clk-am62-dss.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Texas Instruments Incorporated - https://www.ti.com/
+ */
+#include <linux/module.h>
+#include <linux/clk-provider.h>
+#include <linux/platform_device.h>
+
+struct ti_am62_dss_clk {
+	struct clk_hw	hw;
+	unsigned int	div;
+};
+
+#define to_ti_am62_dss_clk(_hw) \
+	container_of(_hw, struct ti_am62_dss_clk, hw)
+
+static unsigned long ti_am62_dss_clk_recalc_rate(struct clk_hw *hw,
+						 unsigned long parent_rate)
+{
+	struct ti_am62_dss_clk *priv = to_ti_am62_dss_clk(hw);
+	unsigned long rate;
+
+	rate = parent_rate;
+	do_div(rate, priv->div);
+	return (unsigned long)rate;
+}
+
+static long ti_am62_dss_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				       unsigned long *prate)
+{
+	struct ti_am62_dss_clk *priv = to_ti_am62_dss_clk(hw);
+
+	if (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) {
+		unsigned long best_parent;
+
+		best_parent = rate * priv->div;
+		*prate = clk_hw_round_rate(clk_hw_get_parent(hw), best_parent);
+	}
+
+	return (*prate / priv->div);
+}
+
+static int ti_am62_dss_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+				    unsigned long parent_rate)
+{
+	/*
+	 * We must report success but we can do so unconditionally because
+	 * ti_am62_dss_clk_round_rate returns values that ensure this call is a
+	 * nop.
+	 */
+
+	return 0;
+}
+
+static const struct clk_ops ti_am62_dss_clk_ops = {
+	.round_rate = ti_am62_dss_clk_round_rate,
+	.set_rate = ti_am62_dss_clk_set_rate,
+	.recalc_rate = ti_am62_dss_clk_recalc_rate,
+};
+
+static struct clk_hw *
+clk_hw_register_am62_dss_clk(struct device *dev, const char *name,
+			     unsigned long flags, unsigned int div)
+{
+	struct ti_am62_dss_clk *priv;
+	struct clk_init_data init = { };
+	struct clk_parent_data pdata = { .index = 0 };
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return ERR_PTR(-ENOMEM);
+
+	/* struct ti_am62_dss_clk assignments */
+	priv->div = div;
+	priv->hw.init = &init;
+
+	init.name = name;
+	init.ops = &ti_am62_dss_clk_ops;
+	init.flags = flags;
+	init.parent_names = NULL;
+	init.parent_data = &pdata;
+	init.num_parents = 1;
+
+	ret = devm_clk_hw_register(dev, &priv->hw);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return &priv->hw;
+}
+
+static void clk_hw_unregister_am62_dss_clk(struct clk_hw *hw)
+{
+	struct ti_am62_dss_clk *priv;
+
+	priv = to_ti_am62_dss_clk(hw);
+
+	clk_hw_unregister(hw);
+	kfree(priv);
+}
+
+static int ti_am62_dss_clk_remove(struct platform_device *pdev)
+{
+	struct clk_hw *hw = platform_get_drvdata(pdev);
+
+	of_clk_del_provider(pdev->dev.of_node);
+	clk_hw_unregister_am62_dss_clk(hw);
+
+	return 0;
+}
+
+static int ti_am62_dss_clk_probe(struct platform_device *pdev)
+{
+	struct clk_hw *hw;
+	const char *clk_name = pdev->name;
+	struct device *dev = &pdev->dev;
+	unsigned long flags = 0;
+	u32 div;
+	int ret;
+
+	if (of_property_read_u32(dev->of_node, "clock-div", &div)) {
+		dev_err(dev, "%s: TI AM62 DSS clock must have a clock-div property.\n",
+			__func__);
+		return -EIO;
+	}
+
+	flags |= CLK_SET_RATE_PARENT;
+
+	hw = clk_hw_register_am62_dss_clk(dev, clk_name, flags, div);
+	if (IS_ERR(hw)) {
+		dev_err(dev, "%s: failed to register %s\n", __func__, clk_name);
+		return PTR_ERR(hw);
+	}
+
+	ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
+	if (ret) {
+		clk_hw_unregister_am62_dss_clk(hw);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, hw);
+
+	return 0;
+}
+
+static const struct of_device_id ti_am62_dss_clk_ids[] = {
+	{ .compatible = "ti,am62-dss-vp0-div-clk" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ti_am62_dss_clk_ids);
+
+static struct platform_driver ti_am62_dss_clk_driver = {
+	.driver = {
+		.name = "ti_am62_dss_clk",
+		.of_match_table = ti_am62_dss_clk_ids,
+	},
+	.probe = ti_am62_dss_clk_probe,
+	.remove = ti_am62_dss_clk_remove,
+};
+module_platform_driver(ti_am62_dss_clk_driver);
+
+MODULE_AUTHOR("Aradhya Bhatia <a-bhatia1@ti.com>");
+MODULE_DESCRIPTION("TI AM62 DSS - OLDI Fixed Clock Divider driver");
+MODULE_LICENSE("GPL");
-- 
2.39.1

Re: [PATCH v2 2/2] clk: keystone: Add support AM62 DSS clock divider

[kernel test robot]

Hi Aradhya,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on linus/master v6.2 next-20230303]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Aradhya-Bhatia/dt-bindings-clock-Add-binding-documentation-for-TI-AM62-DSS-Clock/20230213-200203
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20230213115954.553-3-a-bhatia1%40ti.com
patch subject: [PATCH v2 2/2] clk: keystone: Add support AM62 DSS clock divider
config: arm-randconfig-r046-20230302 (https://download.01.org/0day-ci/archive/20230304/202303040131.k7AUxAg5-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/2ddb7301b11092b73d4a16af06362954f3d5f714
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Aradhya-Bhatia/dt-bindings-clock-Add-binding-documentation-for-TI-AM62-DSS-Clock/20230213-200203
        git checkout 2ddb7301b11092b73d4a16af06362954f3d5f714
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/clk/ mm// net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303040131.k7AUxAg5-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from arch/arm/include/asm/div64.h:107,
                    from include/linux/math.h:6,
                    from include/linux/math64.h:6,
                    from include/linux/time.h:6,
                    from include/linux/stat.h:19,
                    from include/linux/module.h:13,
                    from drivers/clk/keystone/clk-am62-dss.c:5:
   drivers/clk/keystone/clk-am62-dss.c: In function 'ti_am62_dss_clk_recalc_rate':
   include/asm-generic/div64.h:222:35: warning: comparison of distinct pointer types lacks a cast
     222 |         (void)(((typeof((n)) *)0) == ((uint64_t *)0));  \
         |                                   ^~
   drivers/clk/keystone/clk-am62-dss.c:24:9: note: in expansion of macro 'do_div'
      24 |         do_div(rate, priv->div);
         |         ^~~~~~
   In file included from include/linux/build_bug.h:5,
                    from include/linux/container_of.h:5,
                    from include/linux/list.h:5,
                    from include/linux/module.h:12:
   include/asm-generic/div64.h:234:32: warning: right shift count >= width of type [-Wshift-count-overflow]
     234 |         } else if (likely(((n) >> 32) == 0)) {          \
         |                                ^~
   include/linux/compiler.h:77:45: note: in definition of macro 'likely'
      77 | # define likely(x)      __builtin_expect(!!(x), 1)
         |                                             ^
   drivers/clk/keystone/clk-am62-dss.c:24:9: note: in expansion of macro 'do_div'
      24 |         do_div(rate, priv->div);
         |         ^~~~~~
   include/asm-generic/div64.h:238:36: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
     238 |                 __rem = __div64_32(&(n), __base);       \
         |                                    ^~~~
         |                                    |
         |                                    long unsigned int *
   drivers/clk/keystone/clk-am62-dss.c:24:9: note: in expansion of macro 'do_div'
      24 |         do_div(rate, priv->div);
         |         ^~~~~~
   arch/arm/include/asm/div64.h:24:45: note: expected 'uint64_t *' {aka 'long long unsigned int *'} but argument is of type 'long unsigned int *'
      24 | static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
         |                                   ~~~~~~~~~~^
   drivers/clk/keystone/clk-am62-dss.c: In function 'clk_hw_unregister_am62_dss_clk':
>> drivers/clk/keystone/clk-am62-dss.c:99:9: error: implicit declaration of function 'kfree'; did you mean 'kvfree'? [-Werror=implicit-function-declaration]
      99 |         kfree(priv);
         |         ^~~~~
         |         kvfree
   cc1: some warnings being treated as errors


vim +99 drivers/clk/keystone/clk-am62-dss.c

    91	
    92	static void clk_hw_unregister_am62_dss_clk(struct clk_hw *hw)
    93	{
    94		struct ti_am62_dss_clk *priv;
    95	
    96		priv = to_ti_am62_dss_clk(hw);
    97	
    98		clk_hw_unregister(hw);
  > 99		kfree(priv);
   100	}
   101	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests