From: Stephen Boyd <sboyd@kernel.org> To: "A.s. Dong" <aisheng.dong@nxp.com>, "linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org> Cc: "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, "mturquette@baylibre.com" <mturquette@baylibre.com>, "shawnguo@kernel.org" <shawnguo@kernel.org>, Fabio Estevam <fabio.estevam@nxp.com>, dl-linux-imx <linux-imx@nxp.com>, "kernel@pengutronix.de" <kernel@pengutronix.de> Subject: RE: [PATCH V4 11/11] clk: imx: add imx8qxp clk driver Date: Wed, 17 Oct 2018 08:20:15 -0700 [thread overview] Message-ID: <153978961553.5275.18366013902055673508@swboyd.mtv.corp.google.com> (raw) In-Reply-To: <AM0PR04MB4211CA96D332FCC1ED3D547380FF0@AM0PR04MB4211.eurprd04.prod.outlook.com> Quoting A.s. Dong (2018-10-17 02:43:05) > > -----Original Message----- > > From: Stephen Boyd [mailto:sboyd@kernel.org] > > Sent: Wednesday, October 17, 2018 5:39 AM > > To: A.s. Dong <aisheng.dong@nxp.com>; linux-clk@vger.kernel.org > > Cc: linux-arm-kernel@lists.infradead.org; mturquette@baylibre.com; > > shawnguo@kernel.org; Fabio Estevam <fabio.estevam@nxp.com>; dl-linux-imx > > <linux-imx@nxp.com>; kernel@pengutronix.de; A.s. Dong > > <aisheng.dong@nxp.com> > > Subject: Re: [PATCH V4 11/11] clk: imx: add imx8qxp clk driver > > > > Quoting A.s. Dong (2018-10-14 01:08:16) > > > diff --git a/drivers/clk/imx/scu/clk-imx8qxp.c > > > b/drivers/clk/imx/scu/clk-imx8qxp.c > > > new file mode 100644 > > > index 0000000..958c26d > > > --- /dev/null > > > +++ b/drivers/clk/imx/scu/clk-imx8qxp.c > > > > > + > > > +static const char * const enet_sels[] = { "enet_25MHz", > > > +"enet_125MHz", }; static const char * const enet0_rmii_tx_sels[] = { > > > +"enet0_ref_div", "dummy", }; static const char * const > > > +enet1_rmii_tx_sels[] = { "enet1_ref_div", "dummy", }; > > > + > > > +static int imx8qxp_clk_probe(struct platform_device *pdev) { > > > + struct device_node *ccm_node = pdev->dev.of_node; > > > + struct clk_hw **clks; > > > + int ret; > > > + > > > + ret = imx_clk_scu_init(); > > > + if (ret) > > > + return ret; > > > + > > > + clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data) + > > > + sizeof(*clk_data->hws) * > > IMX8QXP_CLK_END, > > > + GFP_KERNEL); > > > + if (!clk_data) > > > + return -ENOMEM; > > > + > > > + clk_data->num = IMX8QXP_CLK_END; > > > + clks = clk_data->hws; > > > + > > > + /* Fixed clocks */ > > > + clks[IMX8QXP_CLK_DUMMY] = > > imx_clk_hw_fixed("dummy", 0); > > > + clks[IMX8QXP_24MHZ] = > > imx_clk_hw_fixed("xtal_24MHz", 24000000); > > > + clks[IMX8QXP_GPT_3M] = > > imx_clk_hw_fixed("gpt_3m", 3000000); > > > + clks[IMX8QXP_32KHZ] = > > imx_clk_hw_fixed("xtal_32KHz", 32768); > > > + > > These are external clocks and should be put in Devicetree. Ok, great! > > > > + /* ARM core */ > > > + clks[IMX8QXP_A35_DIV] = > > imx_clk_divider_scu("a35_div", IMX_SC_R_A35, IMX_SC_PM_CLK_CPU); > > > + > > > + clks[IMX8QXP_IPG_DMA_CLK_ROOT] = > > imx_clk_hw_fixed("ipg_dma_clk_root", 120000000); > > > + clks[IMX8QXP_AXI_CONN_CLK_ROOT] = > > imx_clk_hw_fixed("axi_conn_clk_root", 333333333); > > > + clks[IMX8QXP_AHB_CONN_CLK_ROOT] = > > imx_clk_hw_fixed("ahb_conn_clk_root", 166666666); > > > + clks[IMX8QXP_IPG_CONN_CLK_ROOT] = > > imx_clk_hw_fixed("ipg_conn_clk_root", 83333333); > > > + clks[IMX8QXP_DC_AXI_EXT_CLK] = > > imx_clk_hw_fixed("axi_ext_dc_clk_root", 800000000); > > > + clks[IMX8QXP_DC_AXI_INT_CLK] = > > imx_clk_hw_fixed("axi_int_dc_clk_root", 400000000); > > > + clks[IMX8QXP_DC_CFG_CLK] = > > imx_clk_hw_fixed("cfg_dc_clk_root", 100000000); > > > + clks[IMX8QXP_MIPI_IPG_CLK] = > > imx_clk_hw_fixed("ipg_mipi_clk_root", 120000000); > > > + clks[IMX8QXP_IMG_AXI_CLK] = > > imx_clk_hw_fixed("axi_img_clk_root", 400000000); > > > + clks[IMX8QXP_IMG_IPG_CLK] = > > imx_clk_hw_fixed("ipg_img_clk_root", 200000000); > > > + clks[IMX8QXP_IMG_PXL_CLK] = > > imx_clk_hw_fixed("pxl_img_clk_root", 600000000); > > > + clks[IMX8QXP_HSIO_AXI_CLK] = > > imx_clk_hw_fixed("axi_hsio_clk_root", 400000000); > > > + clks[IMX8QXP_HSIO_PER_CLK] = > > imx_clk_hw_fixed("per_hsio_clk_root", 133333333); > > > > Can these fixed rate clks come from DT? Or what's going on here? > > > > Those are not external clocks. > Do we have to put them in device tree as well? No if they're not external clks then it's fine to keep specifying them in this driver. > > > > + > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id imx8qxp_match[] = { > > > + { .compatible = "fsl,imx8qxp-clk", }, > > > + { /* sentinel */ } > > > +}; > > > + > > > +static struct platform_driver imx8qxp_clk_driver = { > > > + .driver = { > > > + .name = "imx8qxp-clk", > > > + .of_match_table = imx8qxp_match, > > > + }, > > > + .probe = imx8qxp_clk_probe, > > > +}; > > > + > > > +static int __init imx8qxp_clk_init(void) { > > > + return platform_driver_register(&imx8qxp_clk_driver); > > > +} > > > +core_initcall(imx8qxp_clk_init); > > > diff --git a/include/soc/imx/imx8qxp/lpcg.h > > > b/include/soc/imx/imx8qxp/lpcg.h new file mode 100644 index > > > 0000000..afbb5da > > > --- /dev/null > > > +++ b/include/soc/imx/imx8qxp/lpcg.h > > > @@ -0,0 +1,186 @@ > > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > > +/* > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc. > > > + * Copyright 2017~2018 NXP > > > + * > > > + */ > > > + > > > +#ifndef _SC_LPCG_H > > > +#define _SC_LPCG_H > > > + > > > +/*LSIO SS */ > > > +#define PWM_0_LPCG 0x5D400000 > > > > Are these virtual or physical addresses? Because up above they're casted right > > into void __iomem pointers, which makes me thing they're virtual addresses, > > but then that would mean there's some sort of static iomap being used, but > > I'm not aware of anything like that. > > Yes, you're right. Those should be physical address and iomem cast is not necessary. > Sorry about missing that. > Ok. Thanks for fixing it.
WARNING: multiple messages have this Message-ID (diff)
From: sboyd@kernel.org (Stephen Boyd) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH V4 11/11] clk: imx: add imx8qxp clk driver Date: Wed, 17 Oct 2018 08:20:15 -0700 [thread overview] Message-ID: <153978961553.5275.18366013902055673508@swboyd.mtv.corp.google.com> (raw) In-Reply-To: <AM0PR04MB4211CA96D332FCC1ED3D547380FF0@AM0PR04MB4211.eurprd04.prod.outlook.com> Quoting A.s. Dong (2018-10-17 02:43:05) > > -----Original Message----- > > From: Stephen Boyd [mailto:sboyd at kernel.org] > > Sent: Wednesday, October 17, 2018 5:39 AM > > To: A.s. Dong <aisheng.dong@nxp.com>; linux-clk at vger.kernel.org > > Cc: linux-arm-kernel at lists.infradead.org; mturquette at baylibre.com; > > shawnguo at kernel.org; Fabio Estevam <fabio.estevam@nxp.com>; dl-linux-imx > > <linux-imx@nxp.com>; kernel at pengutronix.de; A.s. Dong > > <aisheng.dong@nxp.com> > > Subject: Re: [PATCH V4 11/11] clk: imx: add imx8qxp clk driver > > > > Quoting A.s. Dong (2018-10-14 01:08:16) > > > diff --git a/drivers/clk/imx/scu/clk-imx8qxp.c > > > b/drivers/clk/imx/scu/clk-imx8qxp.c > > > new file mode 100644 > > > index 0000000..958c26d > > > --- /dev/null > > > +++ b/drivers/clk/imx/scu/clk-imx8qxp.c > > > > > + > > > +static const char * const enet_sels[] = { "enet_25MHz", > > > +"enet_125MHz", }; static const char * const enet0_rmii_tx_sels[] = { > > > +"enet0_ref_div", "dummy", }; static const char * const > > > +enet1_rmii_tx_sels[] = { "enet1_ref_div", "dummy", }; > > > + > > > +static int imx8qxp_clk_probe(struct platform_device *pdev) { > > > + struct device_node *ccm_node = pdev->dev.of_node; > > > + struct clk_hw **clks; > > > + int ret; > > > + > > > + ret = imx_clk_scu_init(); > > > + if (ret) > > > + return ret; > > > + > > > + clk_data = devm_kzalloc(&pdev->dev, sizeof(*clk_data) + > > > + sizeof(*clk_data->hws) * > > IMX8QXP_CLK_END, > > > + GFP_KERNEL); > > > + if (!clk_data) > > > + return -ENOMEM; > > > + > > > + clk_data->num = IMX8QXP_CLK_END; > > > + clks = clk_data->hws; > > > + > > > + /* Fixed clocks */ > > > + clks[IMX8QXP_CLK_DUMMY] = > > imx_clk_hw_fixed("dummy", 0); > > > + clks[IMX8QXP_24MHZ] = > > imx_clk_hw_fixed("xtal_24MHz", 24000000); > > > + clks[IMX8QXP_GPT_3M] = > > imx_clk_hw_fixed("gpt_3m", 3000000); > > > + clks[IMX8QXP_32KHZ] = > > imx_clk_hw_fixed("xtal_32KHz", 32768); > > > + > > These are external clocks and should be put in Devicetree. Ok, great! > > > > + /* ARM core */ > > > + clks[IMX8QXP_A35_DIV] = > > imx_clk_divider_scu("a35_div", IMX_SC_R_A35, IMX_SC_PM_CLK_CPU); > > > + > > > + clks[IMX8QXP_IPG_DMA_CLK_ROOT] = > > imx_clk_hw_fixed("ipg_dma_clk_root", 120000000); > > > + clks[IMX8QXP_AXI_CONN_CLK_ROOT] = > > imx_clk_hw_fixed("axi_conn_clk_root", 333333333); > > > + clks[IMX8QXP_AHB_CONN_CLK_ROOT] = > > imx_clk_hw_fixed("ahb_conn_clk_root", 166666666); > > > + clks[IMX8QXP_IPG_CONN_CLK_ROOT] = > > imx_clk_hw_fixed("ipg_conn_clk_root", 83333333); > > > + clks[IMX8QXP_DC_AXI_EXT_CLK] = > > imx_clk_hw_fixed("axi_ext_dc_clk_root", 800000000); > > > + clks[IMX8QXP_DC_AXI_INT_CLK] = > > imx_clk_hw_fixed("axi_int_dc_clk_root", 400000000); > > > + clks[IMX8QXP_DC_CFG_CLK] = > > imx_clk_hw_fixed("cfg_dc_clk_root", 100000000); > > > + clks[IMX8QXP_MIPI_IPG_CLK] = > > imx_clk_hw_fixed("ipg_mipi_clk_root", 120000000); > > > + clks[IMX8QXP_IMG_AXI_CLK] = > > imx_clk_hw_fixed("axi_img_clk_root", 400000000); > > > + clks[IMX8QXP_IMG_IPG_CLK] = > > imx_clk_hw_fixed("ipg_img_clk_root", 200000000); > > > + clks[IMX8QXP_IMG_PXL_CLK] = > > imx_clk_hw_fixed("pxl_img_clk_root", 600000000); > > > + clks[IMX8QXP_HSIO_AXI_CLK] = > > imx_clk_hw_fixed("axi_hsio_clk_root", 400000000); > > > + clks[IMX8QXP_HSIO_PER_CLK] = > > imx_clk_hw_fixed("per_hsio_clk_root", 133333333); > > > > Can these fixed rate clks come from DT? Or what's going on here? > > > > Those are not external clocks. > Do we have to put them in device tree as well? No if they're not external clks then it's fine to keep specifying them in this driver. > > > > + > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id imx8qxp_match[] = { > > > + { .compatible = "fsl,imx8qxp-clk", }, > > > + { /* sentinel */ } > > > +}; > > > + > > > +static struct platform_driver imx8qxp_clk_driver = { > > > + .driver = { > > > + .name = "imx8qxp-clk", > > > + .of_match_table = imx8qxp_match, > > > + }, > > > + .probe = imx8qxp_clk_probe, > > > +}; > > > + > > > +static int __init imx8qxp_clk_init(void) { > > > + return platform_driver_register(&imx8qxp_clk_driver); > > > +} > > > +core_initcall(imx8qxp_clk_init); > > > diff --git a/include/soc/imx/imx8qxp/lpcg.h > > > b/include/soc/imx/imx8qxp/lpcg.h new file mode 100644 index > > > 0000000..afbb5da > > > --- /dev/null > > > +++ b/include/soc/imx/imx8qxp/lpcg.h > > > @@ -0,0 +1,186 @@ > > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > > +/* > > > + * Copyright (C) 2016 Freescale Semiconductor, Inc. > > > + * Copyright 2017~2018 NXP > > > + * > > > + */ > > > + > > > +#ifndef _SC_LPCG_H > > > +#define _SC_LPCG_H > > > + > > > +/*LSIO SS */ > > > +#define PWM_0_LPCG 0x5D400000 > > > > Are these virtual or physical addresses? Because up above they're casted right > > into void __iomem pointers, which makes me thing they're virtual addresses, > > but then that would mean there's some sort of static iomap being used, but > > I'm not aware of anything like that. > > Yes, you're right. Those should be physical address and iomem cast is not necessary. > Sorry about missing that. > Ok. Thanks for fixing it.
next prev parent reply other threads:[~2018-10-17 15:20 UTC|newest] Thread overview: 84+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-14 8:07 [PATCH V4 00/11] clk: imx: add imx8qxp clock support A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-14 8:07 ` [PATCH V4 01/11] clk: imx: add configuration option for mmio clks A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-14 8:07 ` [PATCH V4 02/11] clk: imx: scu: add scu clock common part A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-16 21:31 ` Stephen Boyd 2018-10-16 21:31 ` Stephen Boyd 2018-10-17 9:11 ` A.s. Dong 2018-10-17 9:11 ` A.s. Dong 2018-10-17 15:07 ` Stephen Boyd 2018-10-17 15:07 ` Stephen Boyd 2018-10-17 15:27 ` A.s. Dong 2018-10-17 15:27 ` A.s. Dong 2018-10-14 8:07 ` [PATCH V4 03/11] clk: imx: scu: add scu clock divider A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-16 21:26 ` Stephen Boyd 2018-10-16 21:26 ` Stephen Boyd 2018-10-17 8:56 ` A.s. Dong 2018-10-17 8:56 ` A.s. Dong 2018-10-17 15:17 ` Stephen Boyd 2018-10-17 15:17 ` Stephen Boyd 2018-10-17 15:45 ` A.s. Dong 2018-10-17 15:45 ` A.s. Dong 2018-10-17 16:05 ` Stephen Boyd 2018-10-17 16:05 ` Stephen Boyd 2018-10-18 2:35 ` A.s. Dong 2018-10-18 2:35 ` A.s. Dong 2018-10-14 8:07 ` [PATCH V4 04/11] clk: imx: scu: add scu clock gpr divider A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-16 21:27 ` Stephen Boyd 2018-10-16 21:27 ` Stephen Boyd 2018-10-17 9:03 ` A.s. Dong 2018-10-17 9:03 ` A.s. Dong 2018-10-17 15:17 ` Stephen Boyd 2018-10-17 15:17 ` Stephen Boyd 2018-10-14 8:07 ` [PATCH V4 05/11] clk: imx: scu: add scu clock gate A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-15 7:32 ` Sascha Hauer 2018-10-15 7:32 ` Sascha Hauer 2018-10-15 9:17 ` A.s. Dong 2018-10-15 9:17 ` A.s. Dong 2018-10-15 9:53 ` Sascha Hauer 2018-10-15 9:53 ` Sascha Hauer 2018-10-15 15:30 ` A.s. Dong 2018-10-15 15:30 ` A.s. Dong 2018-10-16 21:18 ` Stephen Boyd 2018-10-16 21:18 ` Stephen Boyd 2018-10-17 7:28 ` A.s. Dong 2018-10-17 7:28 ` A.s. Dong 2018-10-14 8:07 ` [PATCH V4 06/11] clk: imx: scu: add scu clock gpr gate A.s. Dong 2018-10-14 8:07 ` A.s. Dong 2018-10-14 8:08 ` [PATCH V4 07/11] clk: imx: scu: add scu clock mux A.s. Dong 2018-10-14 8:08 ` A.s. Dong 2018-10-14 8:08 ` [PATCH V4 08/11] clk: imx: scu: add scu clock gpr mux A.s. Dong 2018-10-14 8:08 ` A.s. Dong 2018-10-16 21:30 ` Stephen Boyd 2018-10-16 21:30 ` Stephen Boyd 2018-10-17 9:07 ` A.s. Dong 2018-10-17 9:07 ` A.s. Dong 2018-10-17 15:18 ` Stephen Boyd 2018-10-17 15:18 ` Stephen Boyd 2018-10-14 8:08 ` [PATCH V4 09/11] clk: imx: add common imx_clk_hw_fixed functions A.s. Dong 2018-10-14 8:08 ` A.s. Dong 2018-10-16 21:32 ` Stephen Boyd 2018-10-16 21:32 ` Stephen Boyd 2018-10-17 9:21 ` A.s. Dong 2018-10-17 9:21 ` A.s. Dong 2018-10-17 15:18 ` Stephen Boyd 2018-10-17 15:18 ` Stephen Boyd 2018-10-14 8:08 ` [PATCH V4 10/11] clk: imx: add imx_check_clk_hws helper function A.s. Dong 2018-10-14 8:08 ` A.s. Dong 2018-10-16 21:34 ` Stephen Boyd 2018-10-16 21:34 ` Stephen Boyd 2018-10-17 9:24 ` A.s. Dong 2018-10-17 9:24 ` A.s. Dong 2018-10-14 8:08 ` [PATCH V4 11/11] clk: imx: add imx8qxp clk driver A.s. Dong 2018-10-14 8:08 ` A.s. Dong 2018-10-16 21:38 ` Stephen Boyd 2018-10-16 21:38 ` Stephen Boyd 2018-10-17 9:43 ` A.s. Dong 2018-10-17 9:43 ` A.s. Dong 2018-10-17 15:20 ` Stephen Boyd [this message] 2018-10-17 15:20 ` Stephen Boyd
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=153978961553.5275.18366013902055673508@swboyd.mtv.corp.google.com \ --to=sboyd@kernel.org \ --cc=aisheng.dong@nxp.com \ --cc=fabio.estevam@nxp.com \ --cc=kernel@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-imx@nxp.com \ --cc=mturquette@baylibre.com \ --cc=shawnguo@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.