| Message ID | 20230801010234.792557-1-niravkumar.l.rabara@intel.com |
|---|---|
| Headers | show |
| Series | Add support for Agilex5 SoCFPGA platform | expand |
Hi Stephen/Mike, On 7/31/23 20:02, niravkumar.l.rabara@intel.com wrote: > From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> > > Add support for Intel's SoCFPGA Agilex5 platform. The clock manager > driver for the Agilex5 is very similar to the Agilex platform,we can > re-use most of the Agilex clock driver. > > Signed-off-by: Teh Wen Ping <wen.ping.teh@intel.com> > Reviewed-by: Dinh Nguyen <dinguyen@kernel.org> > Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> > --- > drivers/clk/socfpga/clk-agilex.c | 433 ++++++++++++++++++++++++++++++- > 1 file changed, 431 insertions(+), 2 deletions(-) > If you're ok with this patch, can I take this through armsoc? Dinh
Quoting niravkumar.l.rabara@intel.com (2023-07-31 18:02:33) > 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. > + { .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", }, > +}; > + [...] > + > 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. > + 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(). > + if (IS_ERR(base)) > + return PTR_ERR(base); > + > + num_clks = AGILEX5_NUM_CLKS; > + > + clk_data = devm_kzalloc(dev, struct_size(clk_data, clk_data.hws, > + num_clks), GFP_KERNEL); > + if (!clk_data) > + return -ENOMEM; > + > + for (i = 0; i < num_clks; i++) > + clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT); > + > + clk_data->base = base; > + clk_data->clk_data.num = num_clks; > + > + agilex_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), > + clk_data); > + > + agilex_clk_register_c_perip(agilex5_main_perip_c_clks, > + ARRAY_SIZE(agilex5_main_perip_c_clks), > + clk_data); > + > + agilex_clk_register_cnt_perip(agilex5_main_perip_cnt_clks, > + ARRAY_SIZE(agilex5_main_perip_cnt_clks), > + clk_data); > + > + agilex_clk_register_gate(agilex5_gate_clks, > + ARRAY_SIZE(agilex5_gate_clks), clk_data); > + > + of_clk_add_hw_provider(np, of_clk_hw_onecell_get, &clk_data->clk_data); devm? Or when is this provider removed? > + return 0; > +} > + > static int agilex_clkmgr_probe(struct platform_device *pdev) > { > int (*probe_func)(struct platform_device *init_func);
Quoting Dinh Nguyen (2023-08-08 04:03:47) > Hi Stephen/Mike, > > On 7/31/23 20:02, niravkumar.l.rabara@intel.com wrote: > > From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> > > > > Add support for Intel's SoCFPGA Agilex5 platform. The clock manager > > driver for the Agilex5 is very similar to the Agilex platform,we can > > re-use most of the Agilex clock driver. > > > > Signed-off-by: Teh Wen Ping <wen.ping.teh@intel.com> > > Reviewed-by: Dinh Nguyen <dinguyen@kernel.org> > > Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> > > --- > > drivers/clk/socfpga/clk-agilex.c | 433 ++++++++++++++++++++++++++++++- > > 1 file changed, 431 insertions(+), 2 deletions(-) > > > > If you're ok with this patch, can I take this through armsoc? > Usually any binding files go through arm-soc and clk tree but the driver only goes through clk tree via a PR. Is that possible here?
On 8/9/23 16:28, Stephen Boyd wrote: > Quoting Dinh Nguyen (2023-08-08 04:03:47) >> Hi Stephen/Mike, >> >> On 7/31/23 20:02, niravkumar.l.rabara@intel.com wrote: >>> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> >>> >>> Add support for Intel's SoCFPGA Agilex5 platform. The clock manager >>> driver for the Agilex5 is very similar to the Agilex platform,we can >>> re-use most of the Agilex clock driver. >>> >>> Signed-off-by: Teh Wen Ping <wen.ping.teh@intel.com> >>> Reviewed-by: Dinh Nguyen <dinguyen@kernel.org> >>> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> >>> --- >>> drivers/clk/socfpga/clk-agilex.c | 433 ++++++++++++++++++++++++++++++- >>> 1 file changed, 431 insertions(+), 2 deletions(-) >>> >> >> If you're ok with this patch, can I take this through armsoc? >> > > Usually any binding files go through arm-soc and clk tree but the driver > only goes through clk tree via a PR. Is that possible here? Ok. Should be fine in this case. Thanks, Dinh
> -----Original Message----- > From: Stephen Boyd <sboyd@kernel.org> > Sent: Thursday, 10 August, 2023 5:27 AM > To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com> > Cc: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>; 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 <mturquette@baylibre.com>; > 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) > > 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. > > + { .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", }, > > +}; > > + > [...] > > + > > 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. > > > + 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. > > > + if (IS_ERR(base)) > > + return PTR_ERR(base); > > + > > + num_clks = AGILEX5_NUM_CLKS; > > + > > + clk_data = devm_kzalloc(dev, struct_size(clk_data, clk_data.hws, > > + num_clks), GFP_KERNEL); > > + if (!clk_data) > > + return -ENOMEM; > > + > > + for (i = 0; i < num_clks; i++) > > + clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT); > > + > > + clk_data->base = base; > > + clk_data->clk_data.num = num_clks; > > + > > + agilex_clk_register_pll(agilex_pll_clks, ARRAY_SIZE(agilex_pll_clks), > > + clk_data); > > + > > + agilex_clk_register_c_perip(agilex5_main_perip_c_clks, > > + ARRAY_SIZE(agilex5_main_perip_c_clks), > > + clk_data); > > + > > + agilex_clk_register_cnt_perip(agilex5_main_perip_cnt_clks, > > + ARRAY_SIZE(agilex5_main_perip_cnt_clks), > > + clk_data); > > + > > + agilex_clk_register_gate(agilex5_gate_clks, > > + ARRAY_SIZE(agilex5_gate_clks), > > + clk_data); > > + > > + of_clk_add_hw_provider(np, of_clk_hw_onecell_get, > > + &clk_data->clk_data); > > devm? Or when is this provider removed? Will fix in next version. > > > + return 0; > > +} > > + > > static int agilex_clkmgr_probe(struct platform_device *pdev) { > > int (*probe_func)(struct platform_device *init_func);
On 8/13/23 07:53, Rabara, Niravkumar L wrote: > > >> -----Original Message----- >> From: Stephen Boyd <sboyd@kernel.org> >> Sent: Thursday, 10 August, 2023 5:27 AM >> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com> >> Cc: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>; 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 <mturquette@baylibre.com>; >> 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) >>> 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. > >>> + { .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", }, >>> +}; >>> + >> [...] >>> + >>> 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. > >> >>> + 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
> -----Original Message----- > From: Dinh Nguyen <dinguyen@kernel.org> > Sent: Monday, 14 August, 2023 10:48 AM > To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Stephen Boyd > <sboyd@kernel.org> > Cc: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>; andrew@lunn.ch; > conor+dt@kernel.org; devicetree@vger.kernel.org; > krzysztof.kozlowski+dt@linaro.org; linux-clk@vger.kernel.org; linux- > kernel@vger.kernel.org; Turquette, Mike <mturquette@baylibre.com>; > 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 > > > > On 8/13/23 07:53, Rabara, Niravkumar L wrote: > > > > > >> -----Original Message----- > >> From: Stephen Boyd <sboyd@kernel.org> > >> Sent: Thursday, 10 August, 2023 5:27 AM > >> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com> > >> Cc: Ng, Adrian Ho Yin <adrian.ho.yin.ng@intel.com>; 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 <mturquette@baylibre.com>; > >> 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) > >>> 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. > > > >>> + { .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", }, }; > >>> + > >> [...] > >>> + > >>> 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. > > > >> > >>> + 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 Noted Dinh. Thanks, Nirav
From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com> patch [1/5] - Introduced compatible string for Agilex5 board. patch [2/5] - Add Agilex5 reset ID definitions. patch [3/5] - Add Agilex5 clock manager header and yaml file. patch [4/5] - Reused and modified Agilex clock manager driver for Agilex5 to avoid code duplication. This patch depends on patch 4. patch [5/5] - Add device tree files for Agilex5 platform. This patch depends on patch 1,2,3 & 4. patch v2 changes:- - Add separate discription and const for Agilex5 board in yaml file. - Add reset ID definitions required for Agilex5 and reused altr,rst-mgr-s10 bindings similar to Agilex. - Instead of creating separate clock manager driver, re-use agilex clock manager driver and modified it for agilex5 changes to avoid code duplicate. - Fixed device tree alignment issues and other build warnings. Removed ethernet nodes as it will be included in a separate patch. Niravkumar L Rabara (5): dt-bindings: intel: Add Intel Agilex5 compatible dt-bindings: reset: add reset IDs for Agilex5 dt-bindings: clock: add Intel Agilex5 clock manager clk: socfpga: agilex: add clock driver for the Agilex5 arm64: dts: agilex5: add initial support for Intel Agilex5 SoCFPGA .../bindings/arm/intel,socfpga.yaml | 5 + .../bindings/clock/intel,agilex5-clkmgr.yaml | 41 ++ arch/arm64/boot/dts/intel/Makefile | 1 + .../arm64/boot/dts/intel/socfpga_agilex5.dtsi | 468 ++++++++++++++++++ .../boot/dts/intel/socfpga_agilex5_socdk.dts | 39 ++ drivers/clk/socfpga/clk-agilex.c | 433 +++++++++++++++- .../dt-bindings/clock/intel,agilex5-clkmgr.h | 100 ++++ include/dt-bindings/reset/altr,rst-mgr-s10.h | 5 +- 8 files changed, 1089 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/intel,agilex5-clkmgr.yaml create mode 100644 arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi create mode 100644 arch/arm64/boot/dts/intel/socfpga_agilex5_socdk.dts create mode 100644 include/dt-bindings/clock/intel,agilex5-clkmgr.h