On 8/13/23 07:53, Rabara, Niravkumar L wrote:

quoted

-----Original Message-----
From: Stephen Boyd <sboyd@kernel.org>
Sent: Thursday, 10 August, 2023 5:27 AM
To: Rabara, Niravkumar L <redacted>
Cc: Ng, Adrian Ho Yin <redacted>; andrew@lunn.ch;
conor+dt@kernel.org; devicetree@vger.kernel.org; dinguyen@kernel.org;
krzysztof.kozlowski+dt@linaro.org; linux-clk@vger.kernel.org; linux-
kernel@vger.kernel.org; Turquette, Mike [off-list ref];
netdev@vger.kernel.org; p.zabel@pengutronix.de; richardcochran@gmail.com;
robh+dt@kernel.org; wen.ping.teh@intel.com
Subject: Re: [PATCH v2 4/5] clk: socfpga: agilex: add clock driver for the Agilex5

Quoting niravkumar.l.rabara@intel.com (2023-07-31 18:02:33)

quoted

diff --git a/drivers/clk/socfpga/clk-agilex.c

b/drivers/clk/socfpga/clk-agilex.c
index 74d21bd82710..3dcd0f233c17 100644

--- a/drivers/clk/socfpga/clk-agilex.c
+++ b/drivers/clk/socfpga/clk-agilex.c

@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /*
- * Copyright (C) 2019, Intel Corporation
+ * Copyright (C) 2019-2023, Intel Corporation
   */
  #include <linux/slab.h>
  #include <linux/clk-provider.h>

@@ -9,6 +9,7 @@
  #include <linux/platform_device.h>

  #include <dt-bindings/clock/agilex-clock.h>
+#include <dt-bindings/clock/intel,agilex5-clkmgr.h>

  #include "stratix10-clk.h"

@@ -41,6 +42,67 @@ static const struct clk_parent_data mpu_free_mux[] = {
           .name = "f2s-free-clk", },
  };

+static const struct clk_parent_data core0_free_mux[] = {
+       { .fw_name = "main_pll_c1",
+         .name = "main_pll_c1", },

We're adding support for something new? Only set .fw_name in that case, as
.name will never be used. To do even better, set only .index so that we don't do
any string comparisons.

Yes we are adding Agilex5 SoCFPGA platform specific clocks.
I will remove .name and only keep .fw_name in next version of this patch.
  

quoted

quoted

+       { .fw_name = "peri_pll_c0",
+         .name = "peri_pll_c0", },
+       { .fw_name = "osc1",
+         .name = "osc1", },
+       { .fw_name = "cb-intosc-hs-div2-clk",
+         .name = "cb-intosc-hs-div2-clk", },
+       { .fw_name = "f2s-free-clk",
+         .name = "f2s-free-clk", },
+};
+

[...]

quoted

+
  static int n5x_clk_register_c_perip(const struct n5x_perip_c_clock *clks,
                                        int nums, struct
stratix10_clock_data *data)  { @@ -535,6 +917,51 @@ static int
n5x_clkmgr_init(struct platform_device *pdev)
         return 0;
  }

+static int agilex5_clkmgr_init(struct platform_device *pdev) {
+       struct device_node *np = pdev->dev.of_node;
+       struct device *dev = &pdev->dev;
+       struct stratix10_clock_data *clk_data;

Maybe call this stratix_data so that clk_data.clk_data is stratix_data.clk_data.

Will fix this in next version.

quoted

quoted

+       struct resource *res;
+       void __iomem *base;
+       int i, num_clks;
+
+       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       base = devm_ioremap_resource(dev, res);

This is a single function call, devm_platform_ioremap_resource().i

Noted. Will fix in next version.

When you resend a V3, just send this patch. I've already applied the 
other 4 patches.

Dinh