| Message ID | 20220901183343.3188903-1-Mr.Bossman075@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for i.MXRT1170-evk | expand |
On Thu, Sep 1, 2022 at 8:33 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > This patch contains: > - Update to imxrt_defconfig > - Devicetree > - Clock driver > - Pinctrl driver No it does not, I already merged that. I think you should probably split up your series per-subsystem so the clock bindings and changes can be merged separately etc. Then the DTS files can be added to the ARM SoC tree as a final step. When you send everything in one bundle like this subsystem maintainers don't know if they can merge e.g. just the clock patches separately and be done with their part (like what I did with pinctrl). Yours. Linus Walleij
On 9/2/22 04:06, Linus Walleij wrote: > On Thu, Sep 1, 2022 at 8:33 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > >> This patch contains: >> - Update to imxrt_defconfig >> - Devicetree >> - Clock driver >> - Pinctrl driver > > No it does not, I already merged that. > > I think you should probably split up your series per-subsystem so the > clock bindings and changes can be merged separately etc. > > Then the DTS files can be added to the ARM SoC tree as a final step. > > When you send everything in one bundle like this subsystem maintainers > don't know if they can merge e.g. just the clock patches separately > and be done with their part (like what I did with pinctrl). Do you think its possible to add Docs for Device tree compatibles that aren't added yet? Thanks, Jesse Taube > > Yours. > Linus Walleij
On Fri, Sep 2, 2022 at 2:57 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > On 9/2/22 04:06, Linus Walleij wrote: > > On Thu, Sep 1, 2022 at 8:33 PM Jesse Taube <mr.bossman075@gmail.com> wrote: > > > >> This patch contains: > >> - Update to imxrt_defconfig > >> - Devicetree > >> - Clock driver > >> - Pinctrl driver > > > > No it does not, I already merged that. > > > > I think you should probably split up your series per-subsystem so the > > clock bindings and changes can be merged separately etc. > > > > Then the DTS files can be added to the ARM SoC tree as a final step. > > > > When you send everything in one bundle like this subsystem maintainers > > don't know if they can merge e.g. just the clock patches separately > > and be done with their part (like what I did with pinctrl). > > Do you think its possible to add Docs for Device tree compatibles that > aren't added yet? Bindings and drivers are orthogonal, we only submit them together to provide context for reviewers. It is also possible to submit device trees with compatibles and entire nodes without bindings because there essentially is no real police for this. Of course it is not recommended. If you are confident that bindings and device trees will come in the same merge window it is fine to merge them separately through different trees. Yours, Linus Walleij
Quoting Jesse Taube (2022-09-01 11:33:41) > Add clock driver support for i.MXRT1170. > > Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> > --- > V1 -> V2: > - Add slab.h and clock-provider.h > - Add spaces in `root_clocks` > - Expand and sort macro > - Move `clk_hw` structs to `clocks_probe` > - Remove of_irq.h > - Remove unused code/comments > V2 -> V3: > - Expand root_clocks names array > - Remove root_clock_names enum > V3 -> V4: > - Nothing done > V4 -> V5: > - Use __imx_clk_hw_pllv3 to change power bit > V5 -> V6: > - Nothing done > --- > drivers/clk/imx/Kconfig | 7 + > drivers/clk/imx/Makefile | 1 + > drivers/clk/imx/clk-imxrt1170.c | 749 ++++++++++++++++++++++++++++++++ > 3 files changed, 757 insertions(+) > create mode 100644 drivers/clk/imx/clk-imxrt1170.c > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig > index 25785ec9c276..704a7777af4f 100644 > --- a/drivers/clk/imx/Kconfig > +++ b/drivers/clk/imx/Kconfig > @@ -119,3 +119,10 @@ config CLK_IMXRT1050 > select MXC_CLK > help > Build the driver for i.MXRT1050 CCM Clock Driver > + > +config CLK_IMXRT1170 > + tristate "IMXRT1170 CCM Clock Driver" > + depends on SOC_IMXRT > + select MXC_CLK > + help > + Build the driver for i.MXRT1170 CCM Clock Driver > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > index 88b9b9285d22..d607a6d8138a 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -52,4 +52,5 @@ obj-$(CONFIG_CLK_IMX6UL) += clk-imx6ul.o > obj-$(CONFIG_CLK_IMX7D) += clk-imx7d.o > obj-$(CONFIG_CLK_IMX7ULP) += clk-imx7ulp.o > obj-$(CONFIG_CLK_IMXRT1050) += clk-imxrt1050.o > +obj-$(CONFIG_CLK_IMXRT1170) += clk-imxrt1170.o > obj-$(CONFIG_CLK_VF610) += clk-vf610.o > diff --git a/drivers/clk/imx/clk-imxrt1170.c b/drivers/clk/imx/clk-imxrt1170.c > new file mode 100644 > index 000000000000..71d9aacf9751 > --- /dev/null > +++ b/drivers/clk/imx/clk-imxrt1170.c > @@ -0,0 +1,749 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) > +/* > + * Copyright (C) 2022 > + * Author(s): > + * Jesse Taube <Mr.Bossman075@gmail.com> > + */ > +#include <linux/clk.h> Please don't include clk.h unless you use consumer clk APIs. Doesn't look like it is used here? > +#include <linux/of_address.h> > +#include <linux/slab.h> > +#include <linux/clk-provider.h> > +#include <linux/platform_device.h> Sorting alphabetically is nice. > +#include <dt-bindings/clock/imxrt1170-clock.h> > + > +#include "clk.h" > + > +#define CLOCK_MUX_DEFAULT "rcosc48M_div2", "osc", "rcosc400M", "rcosc16M" [...] > + > +static int imxrt1170_clocks_probe(struct platform_device *pdev) > +{ > + void __iomem *ccm_base; > + void __iomem *pll_base; > + struct clk_hw **hws; > + struct clk_hw_onecell_data *clk_hw_data; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct device_node *anp; > + int ret; > + > + clk_hw_data = kzalloc(struct_size(clk_hw_data, hws, > + IMXRT1170_CLK_END), GFP_KERNEL); > + if (WARN_ON(!clk_hw_data)) > + return -ENOMEM; > + > + clk_hw_data->num = IMXRT1170_CLK_END; > + hws = clk_hw_data->hws; > + > + hws[IMXRT1170_CLK_OSC] = imx_obtain_fixed_clk_hw(np, "osc"); > + hws[IMXRT1170_CLK_RCOSC_16M] = imx_obtain_fixed_clk_hw(np, "rcosc16M"); > + hws[IMXRT1170_CLK_OSC_32K] = imx_obtain_fixed_clk_hw(np, "osc32k"); > + > + hws[IMXRT1170_CLK_RCOSC_48M] = imx_clk_hw_fixed_factor("rcosc48M", "rcosc16M", 3, 1); > + hws[IMXRT1170_CLK_RCOSC_400M] = imx_clk_hw_fixed_factor("rcosc400M", "rcosc16M", 25, 1); > + hws[IMXRT1170_CLK_RCOSC_48M_DIV2] = imx_clk_hw_fixed_factor("rcosc48M_div2", "rcosc48M", 1, 2); > + > + anp = of_find_compatible_node(NULL, NULL, "fsl,imxrt-anatop"); > + pll_base = of_iomap(anp, 0); > + of_node_put(anp); > + if (WARN_ON(!pll_base)) > + return -ENOMEM; The kzalloc() leaked. > + > + /* Anatop clocks */ > + hws[IMXRT1170_CLK_DUMMY] = imx_clk_hw_fixed("dummy", 0UL); > + > + hws[IMXRT1170_CLK_PLL_ARM_PRE] = __imx_clk_hw_pllv3(IMX_PLLV3_SYSV2, "pll_arm_pre", "osc", > + pll_base + 0x200, 0xff, 13); > + hws[IMXRT1170_CLK_PLL_ARM_BYPASS] = imx_clk_hw_mux("pll_arm_bypass", pll_base + 0x200, 17, > + 1, pll_arm_mux, 2); > + hws[IMXRT1170_CLK_PLL_ARM_DIV] = clk_hw_register_divider_table(NULL, "pll_arm_div", > + "pll_arm_bypass", CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE, > + pll_base + 0x200, 15, 2, 0, post_div_table, &imx_ccm_lock); > + hws[IMXRT1170_CLK_PLL_ARM] = imx_clk_hw_gate("pll_arm", "pll_arm_div", pll_base + 0x200, 14); > + > + hws[IMXRT1170_CLK_PLL3_PRE] = __imx_clk_hw_pllv3(IMX_PLLV3_GENERICV2, "pll3_pre", "osc", > + pll_base + 0x210, 0x1, 21); > + hws[IMXRT1170_CLK_PLL3_BYPASS] = imx_clk_hw_mux("pll3_bypass", > + pll_base + 0x210, 16, 1, pll3_mux, 2); > + hws[IMXRT1170_CLK_PLL3] = imx_clk_hw_gate("pll3_sys", "pll3_bypass", pll_base + 0x210, 13); > + > + hws[IMXRT1170_CLK_PLL2_PRE] = __imx_clk_hw_pllv3(IMX_PLLV3_GENERICV2, "pll2_pre", "osc", > + pll_base + 0x240, 0x1, 23); > + hws[IMXRT1170_CLK_PLL2_BYPASS] = imx_clk_hw_mux("pll2_bypass", > + pll_base + 0x240, 16, 1, pll2_mux, 2); > + hws[IMXRT1170_CLK_PLL2] = imx_clk_hw_gate("pll2_sys", "pll2_bypass", pll_base + 0x240, 13); > + > + hws[IMXRT1170_CLK_PLL3_PFD0] = imx_clk_hw_pfd("pll3_pfd0", "pll3_sys", pll_base + 0x230, 0); > + hws[IMXRT1170_CLK_PLL3_PFD1] = imx_clk_hw_pfd("pll3_pfd1", "pll3_sys", pll_base + 0x230, 1); > + hws[IMXRT1170_CLK_PLL3_PFD2] = imx_clk_hw_pfd("pll3_pfd2", "pll3_sys", pll_base + 0x230, 2); > + hws[IMXRT1170_CLK_PLL3_PFD3] = imx_clk_hw_pfd("pll3_pfd3", "pll3_sys", pll_base + 0x230, 3); > + hws[IMXRT1170_CLK_PLL3_DIV2_GATE] = imx_clk_hw_fixed_factor("pll3_div2_gate", "pll3_sys", 1, 2); > + hws[IMXRT1170_CLK_PLL3_DIV2] = imx_clk_hw_gate("pll3_div2", "pll3_sys", pll_base + 0x210, 3); > + > + hws[IMXRT1170_CLK_PLL2_PFD0] = imx_clk_hw_pfd("pll2_pfd0", "pll2_sys", pll_base + 0x270, 0); > + hws[IMXRT1170_CLK_PLL2_PFD1] = imx_clk_hw_pfd("pll2_pfd1", "pll2_sys", pll_base + 0x270, 1); > + hws[IMXRT1170_CLK_PLL2_PFD2] = imx_clk_hw_pfd("pll2_pfd2", "pll2_sys", pll_base + 0x270, 2); > + hws[IMXRT1170_CLK_PLL2_PFD3] = imx_clk_hw_pfd("pll2_pfd3", "pll2_sys", pll_base + 0x270, 3); > + > + /* CCM clocks */ > + ccm_base = devm_platform_ioremap_resource(pdev, 0); > + if (WARN_ON(IS_ERR(ccm_base))) > + return PTR_ERR(ccm_base); > + > + hws[IMXRT1170_CLK_M7_SEL] = imx_clk_hw_mux("m7_sel", ccm_base + (1 * 0x80), [....] > + hws[IMXRT1170_CLK_CSI2_UI] = imx_clk_hw_divider("csi2_ui", "csi2_ui_gate", ccm_base + > + (76 * 0x80), 0, 8); > + hws[IMXRT1170_CLK_CSI] = imx_clk_hw_divider("csi", "csi_gate", ccm_base + (77 * 0x80), 0, 8); > + hws[IMXRT1170_CLK_CKO1] = imx_clk_hw_divider("cko1", "cko1_gate", ccm_base + (78 * 0x80), 0, 8); > + hws[IMXRT1170_CLK_CKO2] = imx_clk_hw_divider("cko2", "cko2_gate", ccm_base + (79 * 0x80), 0, 8); > + > + hws[IMXRT1170_CLK_USB] = imx_clk_hw_gate("usb", "bus", ccm_base + LPCG_GATE(115), 0); > + > + imx_check_clk_hws(hws, IMXRT1170_CLK_END); > + > + ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_hw_data); Use devm? Or implement a driver remove function? > + if (ret < 0) { > + dev_err(dev, "Failed to register clks for i.MXRT1170.\n"); > + imx_unregister_hw_clocks(hws, IMXRT1170_CLK_END); > + } > + return ret; > +} > + > +static const struct of_device_id imxrt1170_clk_of_match[] = { > + { .compatible = "fsl,imxrt1170-ccm" }, > + { /* Sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, imxrt1170_clk_of_match); > + > +static struct platform_driver imxrt1170_clk_driver = { > + .probe = imxrt1170_clocks_probe, > + .driver = { > + .name = "imxrt1170-ccm", > + .of_match_table = imxrt1170_clk_of_match, > + },
On 9/30/22 16:28, Stephen Boyd wrote: > Quoting Jesse Taube (2022-09-01 11:33:41) >> Add clock driver support for i.MXRT1170. >> >> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com> >> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com> >> --- >> V1 -> V2: >> - Add slab.h and clock-provider.h >> - Add spaces in `root_clocks` >> - Expand and sort macro >> - Move `clk_hw` structs to `clocks_probe` >> - Remove of_irq.h >> - Remove unused code/comments >> V2 -> V3: >> - Expand root_clocks names array >> - Remove root_clock_names enum >> V3 -> V4: >> - Nothing done >> V4 -> V5: >> - Use __imx_clk_hw_pllv3 to change power bit >> V5 -> V6: >> - Nothing done >> --- >> drivers/clk/imx/Kconfig | 7 + >> drivers/clk/imx/Makefile | 1 + >> drivers/clk/imx/clk-imxrt1170.c | 749 ++++++++++++++++++++++++++++++++ >> 3 files changed, 757 insertions(+) >> create mode 100644 drivers/clk/imx/clk-imxrt1170.c >> >> diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig >> index 25785ec9c276..704a7777af4f 100644 >> --- a/drivers/clk/imx/Kconfig >> +++ b/drivers/clk/imx/Kconfig >> @@ -119,3 +119,10 @@ config CLK_IMXRT1050 >> select MXC_CLK >> help >> Build the driver for i.MXRT1050 CCM Clock Driver >> + >> +config CLK_IMXRT1170 >> + tristate "IMXRT1170 CCM Clock Driver" >> + depends on SOC_IMXRT >> + select MXC_CLK >> + help >> + Build the driver for i.MXRT1170 CCM Clock Driver >> diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile >> index 88b9b9285d22..d607a6d8138a 100644 >> --- a/drivers/clk/imx/Makefile >> +++ b/drivers/clk/imx/Makefile >> @@ -52,4 +52,5 @@ obj-$(CONFIG_CLK_IMX6UL) += clk-imx6ul.o >> obj-$(CONFIG_CLK_IMX7D) += clk-imx7d.o >> obj-$(CONFIG_CLK_IMX7ULP) += clk-imx7ulp.o >> obj-$(CONFIG_CLK_IMXRT1050) += clk-imxrt1050.o >> +obj-$(CONFIG_CLK_IMXRT1170) += clk-imxrt1170.o >> obj-$(CONFIG_CLK_VF610) += clk-vf610.o >> diff --git a/drivers/clk/imx/clk-imxrt1170.c b/drivers/clk/imx/clk-imxrt1170.c >> new file mode 100644 >> index 000000000000..71d9aacf9751 >> --- /dev/null >> +++ b/drivers/clk/imx/clk-imxrt1170.c >> @@ -0,0 +1,749 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) >> +/* >> + * Copyright (C) 2022 >> + * Author(s): >> + * Jesse Taube <Mr.Bossman075@gmail.com> >> + */ >> +#include <linux/clk.h> > > Please don't include clk.h unless you use consumer clk APIs. Doesn't > look like it is used here? > >> +#include <linux/of_address.h> >> +#include <linux/slab.h> >> +#include <linux/clk-provider.h> >> +#include <linux/platform_device.h> > > Sorting alphabetically is nice. Oh my bad, will fix. >> +#include <dt-bindings/clock/imxrt1170-clock.h> >> + >> +#include "clk.h" >> + >> +#define CLOCK_MUX_DEFAULT "rcosc48M_div2", "osc", "rcosc400M", "rcosc16M" > [...] >> + >> +static int imxrt1170_clocks_probe(struct platform_device *pdev) >> +{ >> + void __iomem *ccm_base; >> + void __iomem *pll_base; >> + struct clk_hw **hws; >> + struct clk_hw_onecell_data *clk_hw_data; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; >> + struct device_node *anp; >> + int ret; >> + >> + clk_hw_data = kzalloc(struct_size(clk_hw_data, hws, >> + IMXRT1170_CLK_END), GFP_KERNEL); >> + if (WARN_ON(!clk_hw_data)) >> + return -ENOMEM; >> + >> + clk_hw_data->num = IMXRT1170_CLK_END; >> + hws = clk_hw_data->hws; >> + >> + hws[IMXRT1170_CLK_OSC] = imx_obtain_fixed_clk_hw(np, "osc"); >> + hws[IMXRT1170_CLK_RCOSC_16M] = imx_obtain_fixed_clk_hw(np, "rcosc16M"); >> + hws[IMXRT1170_CLK_OSC_32K] = imx_obtain_fixed_clk_hw(np, "osc32k"); >> + >> + hws[IMXRT1170_CLK_RCOSC_48M] = imx_clk_hw_fixed_factor("rcosc48M", "rcosc16M", 3, 1); >> + hws[IMXRT1170_CLK_RCOSC_400M] = imx_clk_hw_fixed_factor("rcosc400M", "rcosc16M", 25, 1); >> + hws[IMXRT1170_CLK_RCOSC_48M_DIV2] = imx_clk_hw_fixed_factor("rcosc48M_div2", "rcosc48M", 1, 2); >> + >> + anp = of_find_compatible_node(NULL, NULL, "fsl,imxrt-anatop"); >> + pll_base = of_iomap(anp, 0); >> + of_node_put(anp); >> + if (WARN_ON(!pll_base)) >> + return -ENOMEM; > > The kzalloc() leaked. LOL `grep -r of_find_compatible_node drivers/clk/imx`... Shall I send patches for the rest of IMX. > >> + >> + /* Anatop clocks */ >> + hws[IMXRT1170_CLK_DUMMY] = imx_clk_hw_fixed("dummy", 0UL); >> + >> + hws[IMXRT1170_CLK_PLL_ARM_PRE] = __imx_clk_hw_pllv3(IMX_PLLV3_SYSV2, "pll_arm_pre", "osc", >> + pll_base + 0x200, 0xff, 13); >> + hws[IMXRT1170_CLK_PLL_ARM_BYPASS] = imx_clk_hw_mux("pll_arm_bypass", pll_base + 0x200, 17, >> + 1, pll_arm_mux, 2); >> + hws[IMXRT1170_CLK_PLL_ARM_DIV] = clk_hw_register_divider_table(NULL, "pll_arm_div", >> + "pll_arm_bypass", CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE, >> + pll_base + 0x200, 15, 2, 0, post_div_table, &imx_ccm_lock); >> + hws[IMXRT1170_CLK_PLL_ARM] = imx_clk_hw_gate("pll_arm", "pll_arm_div", pll_base + 0x200, 14); >> + >> + hws[IMXRT1170_CLK_PLL3_PRE] = __imx_clk_hw_pllv3(IMX_PLLV3_GENERICV2, "pll3_pre", "osc", >> + pll_base + 0x210, 0x1, 21); >> + hws[IMXRT1170_CLK_PLL3_BYPASS] = imx_clk_hw_mux("pll3_bypass", >> + pll_base + 0x210, 16, 1, pll3_mux, 2); >> + hws[IMXRT1170_CLK_PLL3] = imx_clk_hw_gate("pll3_sys", "pll3_bypass", pll_base + 0x210, 13); >> + >> + hws[IMXRT1170_CLK_PLL2_PRE] = __imx_clk_hw_pllv3(IMX_PLLV3_GENERICV2, "pll2_pre", "osc", >> + pll_base + 0x240, 0x1, 23); >> + hws[IMXRT1170_CLK_PLL2_BYPASS] = imx_clk_hw_mux("pll2_bypass", >> + pll_base + 0x240, 16, 1, pll2_mux, 2); >> + hws[IMXRT1170_CLK_PLL2] = imx_clk_hw_gate("pll2_sys", "pll2_bypass", pll_base + 0x240, 13); >> + >> + hws[IMXRT1170_CLK_PLL3_PFD0] = imx_clk_hw_pfd("pll3_pfd0", "pll3_sys", pll_base + 0x230, 0); >> + hws[IMXRT1170_CLK_PLL3_PFD1] = imx_clk_hw_pfd("pll3_pfd1", "pll3_sys", pll_base + 0x230, 1); >> + hws[IMXRT1170_CLK_PLL3_PFD2] = imx_clk_hw_pfd("pll3_pfd2", "pll3_sys", pll_base + 0x230, 2); >> + hws[IMXRT1170_CLK_PLL3_PFD3] = imx_clk_hw_pfd("pll3_pfd3", "pll3_sys", pll_base + 0x230, 3); >> + hws[IMXRT1170_CLK_PLL3_DIV2_GATE] = imx_clk_hw_fixed_factor("pll3_div2_gate", "pll3_sys", 1, 2); >> + hws[IMXRT1170_CLK_PLL3_DIV2] = imx_clk_hw_gate("pll3_div2", "pll3_sys", pll_base + 0x210, 3); >> + >> + hws[IMXRT1170_CLK_PLL2_PFD0] = imx_clk_hw_pfd("pll2_pfd0", "pll2_sys", pll_base + 0x270, 0); >> + hws[IMXRT1170_CLK_PLL2_PFD1] = imx_clk_hw_pfd("pll2_pfd1", "pll2_sys", pll_base + 0x270, 1); >> + hws[IMXRT1170_CLK_PLL2_PFD2] = imx_clk_hw_pfd("pll2_pfd2", "pll2_sys", pll_base + 0x270, 2); >> + hws[IMXRT1170_CLK_PLL2_PFD3] = imx_clk_hw_pfd("pll2_pfd3", "pll2_sys", pll_base + 0x270, 3); >> + >> + /* CCM clocks */ >> + ccm_base = devm_platform_ioremap_resource(pdev, 0); >> + if (WARN_ON(IS_ERR(ccm_base))) >> + return PTR_ERR(ccm_base); >> + >> + hws[IMXRT1170_CLK_M7_SEL] = imx_clk_hw_mux("m7_sel", ccm_base + (1 * 0x80), > [....] >> + hws[IMXRT1170_CLK_CSI2_UI] = imx_clk_hw_divider("csi2_ui", "csi2_ui_gate", ccm_base + >> + (76 * 0x80), 0, 8); >> + hws[IMXRT1170_CLK_CSI] = imx_clk_hw_divider("csi", "csi_gate", ccm_base + (77 * 0x80), 0, 8); >> + hws[IMXRT1170_CLK_CKO1] = imx_clk_hw_divider("cko1", "cko1_gate", ccm_base + (78 * 0x80), 0, 8); >> + hws[IMXRT1170_CLK_CKO2] = imx_clk_hw_divider("cko2", "cko2_gate", ccm_base + (79 * 0x80), 0, 8); >> + >> + hws[IMXRT1170_CLK_USB] = imx_clk_hw_gate("usb", "bus", ccm_base + LPCG_GATE(115), 0); >> + >> + imx_check_clk_hws(hws, IMXRT1170_CLK_END); >> + >> + ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_hw_data); > > Use devm? Or implement a driver remove function? Uh this is the same in the rest of imx could you explain a bit more? Very sorry for the low quality code :( Thanks, Jesse Taube >> + if (ret < 0) { >> + dev_err(dev, "Failed to register clks for i.MXRT1170.\n"); >> + imx_unregister_hw_clocks(hws, IMXRT1170_CLK_END); >> + } >> + return ret; >> +} >> + >> +static const struct of_device_id imxrt1170_clk_of_match[] = { >> + { .compatible = "fsl,imxrt1170-ccm" }, >> + { /* Sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(of, imxrt1170_clk_of_match); >> + >> +static struct platform_driver imxrt1170_clk_driver = { >> + .probe = imxrt1170_clocks_probe, >> + .driver = { >> + .name = "imxrt1170-ccm", >> + .of_match_table = imxrt1170_clk_of_match, >> + },
Quoting Jesse Taube (2022-10-01 09:15:38) > On 9/30/22 16:28, Stephen Boyd wrote: > > Quoting Jesse Taube (2022-09-01 11:33:41) > >> + > >> + anp = of_find_compatible_node(NULL, NULL, "fsl,imxrt-anatop"); > >> + pll_base = of_iomap(anp, 0); > >> + of_node_put(anp); > >> + if (WARN_ON(!pll_base)) > >> + return -ENOMEM; > > > > The kzalloc() leaked. > LOL `grep -r of_find_compatible_node drivers/clk/imx`... > Shall I send patches for the rest of IMX. > Sure? Introducing more things to cleanup isn't useful, that's all. If you have other things to do then don't worry about it. > >> + > >> + imx_check_clk_hws(hws, IMXRT1170_CLK_END); > >> + > >> + ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_hw_data); > > > > Use devm? Or implement a driver remove function? > Uh this is the same in the rest of imx could you explain a bit more? > Use devm_of_clk_add_hw_provider() so that the provider is removed on driver unbind.