RE: [PATCH 04/23] clk: samsung: fsd: Add cmu_peric block clock information

["Alim Akhtar" <alim.akhtar@samsung.com>]

>-----Original Message-----
>From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@canonical.com]
>Sent: Thursday, January 13, 2022 6:25 PM
>To: Alim Akhtar <alim.akhtar@samsung.com>; linux-arm-
>kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>Cc: soc@kernel.org; linux-clk@vger.kernel.org; devicetree@vger.kernel.org;
>olof@lixom.net; linus.walleij@linaro.org; catalin.marinas@arm.com;
>robh+dt@kernel.org; s.nawrocki@samsung.com; linux-samsung-
>soc@vger.kernel.org; pankaj.dubey@samsung.com; linux-fsd@tesla.com;
>Aswani Reddy <aswani.reddy@samsung.com>; Niyas Ahmed S T
><niyas.ahmed@samsung.com>; Chandrasekar R <rcsekar@samsung.com>;
>Jayati Sahu <jayati.sahu@samsung.com>; Sriranjani P
><sriranjani.p@samsung.com>; Ajay Kumar <ajaykumar.rs@samsung.com>
>Subject: Re: [PATCH 04/23] clk: samsung: fsd: Add cmu_peric block clock
>information
>
>On 13/01/2022 13:11, Alim Akhtar wrote:
>> This patch adds CMU_PERIC block clock information needed for various
>> IPs functions found in this block.
>
>Here and in all other commits, please do not use "This patch". Instead:
>https://protect2.fireeye.com/v1/url?k=5ec41fe1-015f26dc-5ec594ae-
>0cc47a31309a-72c796795ac37ef5&q=1&e=2a1e171b-f066-48ff-95a7-
>12605dbbf8a9&u=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Fv5.13%2Fso
>urce%2FDocumentation%2Fprocess%2Fsubmitting-patches.rst%23L89
>
Noted
>>
>> Cc: linux-fsd@tesla.com
>> Signed-off-by: Aswani Reddy <aswani.reddy@samsung.com>
>> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
>> Signed-off-by: Chandrasekar R <rcsekar@samsung.com>
>> Signed-off-by: Jayati Sahu <jayati.sahu@samsung.com>
>> Signed-off-by: Sriranjani P <sriranjani.p@samsung.com>
>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-fsd.c | 464
>> +++++++++++++++++++++++++++++++++-
>>  1 file changed, 463 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-fsd.c
>> b/drivers/clk/samsung/clk-fsd.c index e47523106d9e..6da20966ba99
>> 100644
>> --- a/drivers/clk/samsung/clk-fsd.c
>> +++ b/drivers/clk/samsung/clk-fsd.c
>> @@ -9,12 +9,59 @@
>>   *
>>   */
>>
>> -#include <linux/clk-provider.h>
>>  #include <linux/of.h>
>> +#include <linux/clk.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/platform_device.h>
>
>Please order the includes alphabetically.
>
Sure will fix this
>>
>>  #include "clk.h"
>>  #include <dt-bindings/clock/fsd-clk.h>
>>
>> +/* Gate register bits */
>> +#define GATE_MANUAL		BIT(20)
>> +#define GATE_ENABLE_HWACG	BIT(28)
>> +
>> +/* Gate register offsets range */
>> +#define GATE_OFF_START		0x2000
>> +#define GATE_OFF_END		0x2fff
>> +
>> +/**
>> + * fsd_init_clocks - Set clocks initial configuration
>> + * @np:			CMU device tree node with "reg" property
>(CMU addr)
>> + * @reg_offs:		Register offsets array for clocks to init
>> + * @reg_offs_len:	Number of register offsets in reg_offs array
>> + *
>> + * Set manual control mode for all gate clocks.
>> + */
>> +static void __init fsd_init_clocks(struct device_node *np,
>> +		const unsigned long *reg_offs, size_t reg_offs_len)
>
>The same as exynos_arm64_init_clocks - please re-use instead of duplicating.
>
Will re-base on latest tree to have these common functions
>> +{
>> +	void __iomem *reg_base;
>> +	size_t i;
>> +
>> +	reg_base = of_iomap(np, 0);
>> +	if (!reg_base)
>> +		panic("%s: failed to map registers\n", __func__);
>> +
>> +	for (i = 0; i < reg_offs_len; ++i) {
>> +		void __iomem *reg = reg_base + reg_offs[i];
>> +		u32 val;
>> +
>> +		/* Modify only gate clock registers */
>> +		if (reg_offs[i] < GATE_OFF_START || reg_offs[i] >
>GATE_OFF_END)
>> +			continue;
>> +
>> +		val = readl(reg);
>> +		val |= GATE_MANUAL;
>> +		val &= ~GATE_ENABLE_HWACG;
>> +		writel(val, reg);
>> +	}
>> +
>> +	iounmap(reg_base);
>> +}
>> +
>
>(...)
>
>> +/**
>> + * fsd_cmu_probe - Probe function for FSD platform clocks
>> + * @pdev: Pointer to platform device
>> + *
>> + * Configure clock hierarchy for clock domains of FSD platform  */
>> +static int __init fsd_cmu_probe(struct platform_device *pdev) {
>> +	const struct samsung_cmu_info *info;
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np = dev->of_node;
>> +
>> +	info = of_device_get_match_data(dev);
>> +	fsd_init_clocks(np, info->clk_regs, info->nr_clk_regs);
>> +	samsung_cmu_register_one(np, info);
>> +
>> +	/* Keep bus clock running, so it's possible to access CMU registers */
>> +	if (info->clk_name) {
>> +		struct clk *bus_clk;
>> +
>> +		bus_clk = clk_get(dev, info->clk_name);
>> +		if (IS_ERR(bus_clk)) {
>> +			pr_err("%s: could not find bus clock %s; err = %ld\n",
>> +			       __func__, info->clk_name, PTR_ERR(bus_clk));
>> +		} else {
>> +			clk_prepare_enable(bus_clk);
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>
>Please re-use exynos_arm64_register_cmu(). This will also solve my previous
>comment about exynos_arm64_init_clocks().
>
ok
>> +
>> +/* CMUs which belong to Power Domains and need runtime PM to be
>> +implemented */ static const struct of_device_id fsd_cmu_of_match[] = {
>> +	{
>> +		.compatible = "tesla,fsd-clock-peric",
>> +		.data = &peric_cmu_info,
>> +	}, {
>> +	},
>> +};
>> +
>> +static struct platform_driver fsd_cmu_driver __refdata = {
>> +	.driver	= {
>> +		.name = "fsd-cmu",
>> +		.of_match_table = fsd_cmu_of_match,
>> +		.suppress_bind_attrs = true,
>> +	},
>> +	.probe = fsd_cmu_probe,
>> +};
>> +
>> +static int __init fsd_cmu_init(void)
>> +{
>> +	return platform_driver_register(&fsd_cmu_driver);
>> +}
>> +core_initcall(fsd_cmu_init);
>>
>
>
>Best regards,
>Krzysztof