Message ID | 1544662938-5020-3-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | clk: imx: add imx8qxp clock support | expand |
Quoting Aisheng Dong (2018-12-12 17:07:55) > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig > index 37478ba..5c26f4e 100644 > --- a/drivers/clk/imx/Kconfig > +++ b/drivers/clk/imx/Kconfig > @@ -3,3 +3,9 @@ > config MXC_CLK > bool > def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && SOC_IMX8MQ) > + > +config MXC_CLK_SCU > + bool "IMX SCU Clock" > + depends on ARCH_MXC && ARM64 This should depend on the IMX_SCU config too, right? Otherwise compilation will fail. Plus, I can't seem to compile this driver regardless, because the defines for the CLK messages are in some header file that is just included in another header file, and that is all going through another tree. Why? Is there any reason why those defines can't just be written in the clk-scu.h file introduced in this patch?
> -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] > Quoting Aisheng Dong (2018-12-12 17:07:55) > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index > > 37478ba..5c26f4e 100644 > > --- a/drivers/clk/imx/Kconfig > > +++ b/drivers/clk/imx/Kconfig > > @@ -3,3 +3,9 @@ > > config MXC_CLK > > bool > > def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && > > SOC_IMX8MQ) > > + > > +config MXC_CLK_SCU > > + bool "IMX SCU Clock" > > + depends on ARCH_MXC && ARM64 > > This should depend on the IMX_SCU config too, right? Otherwise compilation > will fail. > Yes, you're right. I will add it. > Plus, I can't seem to compile this driver regardless, because the defines for the > CLK messages are in some header file that is just included in another header > file, and that is all going through another tree. Why? > > Is there any reason why those defines can't just be written in the clk-scu.h file > introduced in this patch? Sorry about that. Yes, it depends on PM service headfile [1] which defines all the power related service together by SCU protocol, including power domain/clk/reset and etc. (we probably should divide them to get a better separation from the start). But Patch 4 depends on the resource ID headfile shared by most SCU client users. [1] https://patchwork.kernel.org/patch/10664137/ [2] https://patchwork.kernel.org/patch/10664125/ How would you suggest for this situation? Sorry for the inconvenience. Regards Dong Aisheng
Quoting Aisheng Dong (2018-12-13 03:19:26) > > -----Original Message----- > > From: Stephen Boyd [mailto:sboyd@kernel.org] > > Quoting Aisheng Dong (2018-12-12 17:07:55) > > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index > > > 37478ba..5c26f4e 100644 > > > --- a/drivers/clk/imx/Kconfig > > > +++ b/drivers/clk/imx/Kconfig > > > @@ -3,3 +3,9 @@ > > > config MXC_CLK > > > bool > > > def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && > > > SOC_IMX8MQ) > > > + > > > +config MXC_CLK_SCU > > > + bool "IMX SCU Clock" > > > + depends on ARCH_MXC && ARM64 > > > > This should depend on the IMX_SCU config too, right? Otherwise compilation > > will fail. > > > > Yes, you're right. I will add it. > > > Plus, I can't seem to compile this driver regardless, because the defines for the > > CLK messages are in some header file that is just included in another header > > file, and that is all going through another tree. Why? > > > > Is there any reason why those defines can't just be written in the clk-scu.h file > > introduced in this patch? > > Sorry about that. > Yes, it depends on PM service headfile [1] which defines all the power related service together > by SCU protocol, including power domain/clk/reset and etc. > (we probably should divide them to get a better separation from the start). > But Patch 4 depends on the resource ID headfile shared by most SCU client users. > > [1] https://patchwork.kernel.org/patch/10664137/ > [2] https://patchwork.kernel.org/patch/10664125/ > > How would you suggest for this situation? > I suggest the defines just go into the clk driver. Is that a workable solution or does something not work?
> -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] > Sent: Friday, December 14, 2018 9:31 AM > To: linux-clk@vger.kernel.org; Aisheng Dong <aisheng.dong@nxp.com> > 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 > Subject: RE: [PATCH V11 2/7] clk: imx: add scu clock common part > > Quoting Aisheng Dong (2018-12-13 03:19:26) > > > -----Original Message----- > > > From: Stephen Boyd [mailto:sboyd@kernel.org] Quoting Aisheng Dong > > > (2018-12-12 17:07:55) > > > > diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig > > > > index 37478ba..5c26f4e 100644 > > > > --- a/drivers/clk/imx/Kconfig > > > > +++ b/drivers/clk/imx/Kconfig > > > > @@ -3,3 +3,9 @@ > > > > config MXC_CLK > > > > bool > > > > def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 > && > > > > SOC_IMX8MQ) > > > > + > > > > +config MXC_CLK_SCU > > > > + bool "IMX SCU Clock" > > > > + depends on ARCH_MXC && ARM64 > > > > > > This should depend on the IMX_SCU config too, right? Otherwise > > > compilation will fail. > > > > > > > Yes, you're right. I will add it. > > > > > Plus, I can't seem to compile this driver regardless, because the > > > defines for the CLK messages are in some header file that is just > > > included in another header file, and that is all going through another tree. > Why? > > > > > > Is there any reason why those defines can't just be written in the > > > clk-scu.h file introduced in this patch? > > > > Sorry about that. > > Yes, it depends on PM service headfile [1] which defines all the power > > related service together by SCU protocol, including power domain/clk/reset > and etc. > > (we probably should divide them to get a better separation from the start). > > But Patch 4 depends on the resource ID headfile shared by most SCU client > users. > > > > [1] > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat > > > chwork.kernel.org%2Fpatch%2F10664137%2F&data=02%7C01%7Caishe > ng.don > > > g%40nxp.com%7Cea09bbc7dfbf424844bb08d66163c240%7C686ea1d3bc2b4 > c6fa92cd > > > 99c5c301635%7C0%7C0%7C636803478422100123&sdata=GwOYjZns%2 > BJxLePnT5 > > 6ZTzRuME3DQA5V8eukvRXCf1s0%3D&reserved=0 > > [2] > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat > > > chwork.kernel.org%2Fpatch%2F10664125%2F&data=02%7C01%7Caishe > ng.don > > > g%40nxp.com%7Cea09bbc7dfbf424844bb08d66163c240%7C686ea1d3bc2b4 > c6fa92cd > > > 99c5c301635%7C0%7C0%7C636803478422100123&sdata=CJGAlaF4K3 > uK3bYlEbC > > sSRQssa25%2F%2BiPaI5y4x7grRc%3D&reserved=0 > > > > How would you suggest for this situation? > > > > I suggest the defines just go into the clk driver. Is that a workable solution or > does something not work? Are you suggesting me to doing it now? As the patch is already in Shawn's tree, but I think I can do it and clean up the file in Shawn's tree later. However, only moving the clk related defines into clk driver is not enough. The driver compiling still depends on the resource ID file which is shared by power domain driver and clk driver (and possible many others). So we still have dependency issues. Any suggestion? Regards Dong Aisheng
Quoting Aisheng Dong (2018-12-13 17:44:55) > > > > > > How would you suggest for this situation? > > > > > > > I suggest the defines just go into the clk driver. Is that a workable solution or > > does something not work? > > Are you suggesting me to doing it now? > As the patch is already in Shawn's tree, but I think I can do it and clean up > the file in Shawn's tree later. > > However, only moving the clk related defines into clk driver is not enough. > The driver compiling still depends on the resource ID file which is shared > by power domain driver and clk driver (and possible many others). > So we still have dependency issues. > Do the resources need to be defined in one place either? Or can they be spread to all the drivers that use them?
> -----Original Message----- > From: Stephen Boyd [mailto:sboyd@kernel.org] [...] > Quoting Aisheng Dong (2018-12-13 17:44:55) > > > > > > > > How would you suggest for this situation? > > > > > > > > > > I suggest the defines just go into the clk driver. Is that a > > > workable solution or does something not work? > > > > Are you suggesting me to doing it now? > > As the patch is already in Shawn's tree, but I think I can do it and > > clean up the file in Shawn's tree later. > > > > However, only moving the clk related defines into clk driver is not enough. > > The driver compiling still depends on the resource ID file which is > > shared by power domain driver and clk driver (and possible many others). > > So we still have dependency issues. > > > > Do the resources need to be defined in one place either? Or can they be spread > to all the drivers that use them? I guess no. They're mostly shared among a variety of driver, power domain, clk and other possible clients. e.g. power domain use all of them to specify domain in device tree. + adma_lpuart0: serial@5a060000 { + compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart"; + reg = <0x5a060000 0x1000>; + interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>; + interrupt-parent = <&gic>; + clocks = <&adma_lpcg IMX8QXP_ADMA_LPCG_UART0_BAUD_CLK>; + clock-names = "ipg"; + power-domains = <&pd IMX_SC_R_UART_0>; + status = "disabled"; + }; https://patchwork.kernel.org/patch/10677311/ Regards Dong Aisheng
diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index 37478ba..5c26f4e 100644 --- a/drivers/clk/imx/Kconfig +++ b/drivers/clk/imx/Kconfig @@ -3,3 +3,9 @@ config MXC_CLK bool def_bool (ARCH_MXC && !ARM64) || (ARCH_MXC && ARM64 && SOC_IMX8MQ) + +config MXC_CLK_SCU + bool "IMX SCU Clock" + depends on ARCH_MXC && ARM64 + help + Build the driver for IMX SCU based clocks. diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index f850424..4abed37 100644 --- a/drivers/clk/imx/Makefile +++ b/drivers/clk/imx/Makefile @@ -20,6 +20,9 @@ obj-$(CONFIG_MXC_CLK) += \ clk-pllv4.o \ clk-sccg-pll.o +obj-$(CONFIG_MXC_CLK_SCU) += \ + clk-scu.o + obj-$(CONFIG_SOC_IMX1) += clk-imx1.o obj-$(CONFIG_SOC_IMX21) += clk-imx21.o obj-$(CONFIG_SOC_IMX25) += clk-imx25.o diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c new file mode 100644 index 0000000..2987deb --- /dev/null +++ b/drivers/clk/imx/clk-scu.c @@ -0,0 +1,270 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2018 NXP + * Dong Aisheng <aisheng.dong@nxp.com> + */ + +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/slab.h> + +#include "clk-scu.h" + +struct imx_sc_ipc *ccm_ipc_handle; + +/* + * struct clk_scu - Description of one SCU clock + * @hw: the common clk_hw + * @rsrc_id: resource ID of this SCU clock + * @clk_type: type of this clock resource + */ +struct clk_scu { + struct clk_hw hw; + u16 rsrc_id; + u8 clk_type; +}; + +/* + * struct imx_sc_msg_req_set_clock_rate - clock set rate protocol + * @hdr: SCU protocol header + * @rate: rate to set + * @resource: clock resource to set rate + * @clk: clk type of this resource + * + * This structure describes the SCU protocol of clock rate set + */ +struct imx_sc_msg_req_set_clock_rate { + struct imx_sc_rpc_msg hdr; + __le32 rate; + __le16 resource; + u8 clk; +} __packed; + +struct req_get_clock_rate { + __le16 resource; + u8 clk; +} __packed; + +struct resp_get_clock_rate { + __le32 rate; +}; + +/* + * struct imx_sc_msg_get_clock_rate - clock get rate protocol + * @hdr: SCU protocol header + * @req: get rate request protocol + * @resp: get rate response protocol + * + * This structure describes the SCU protocol of clock rate get + */ +struct imx_sc_msg_get_clock_rate { + struct imx_sc_rpc_msg hdr; + union { + struct req_get_clock_rate req; + struct resp_get_clock_rate resp; + } data; +}; + +/* + * struct imx_sc_msg_req_clock_enable - clock gate protocol + * @hdr: SCU protocol header + * @resource: clock resource to gate + * @clk: clk type of this resource + * @enable: whether gate off the clock + * @autog: HW auto gate enable + * + * This structure describes the SCU protocol of clock gate + */ +struct imx_sc_msg_req_clock_enable { + struct imx_sc_rpc_msg hdr; + __le16 resource; + u8 clk; + u8 enable; + u8 autog; +} __packed; + +static inline struct clk_scu *to_clk_scu(struct clk_hw *hw) +{ + return container_of(hw, struct clk_scu, hw); +} + +int imx_clk_scu_init(void) +{ + return imx_scu_get_handle(&ccm_ipc_handle); +} + +/* + * clk_scu_recalc_rate - Get clock rate for a SCU clock + * @hw: clock to get rate for + * @parent_rate: parent rate provided by common clock framework, not used + * + * Gets the current clock rate of a SCU clock. Returns the current + * clock rate, or zero in failure. + */ +static unsigned long clk_scu_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct clk_scu *clk = to_clk_scu(hw); + struct imx_sc_msg_get_clock_rate msg; + struct imx_sc_rpc_msg *hdr = &msg.hdr; + int ret; + + hdr->ver = IMX_SC_RPC_VERSION; + hdr->svc = IMX_SC_RPC_SVC_PM; + hdr->func = IMX_SC_PM_FUNC_GET_CLOCK_RATE; + hdr->size = 2; + + msg.data.req.resource = cpu_to_le16(clk->rsrc_id); + msg.data.req.clk = clk->clk_type; + + ret = imx_scu_call_rpc(ccm_ipc_handle, &msg, true); + if (ret) { + pr_err("%s: failed to get clock rate %d\n", + clk_hw_get_name(hw), ret); + return 0; + } + + return le32_to_cpu(msg.data.resp.rate); +} + +/* + * clk_scu_round_rate - Round clock rate for a SCU clock + * @hw: clock to round rate for + * @rate: rate to round + * @parent_rate: parent rate provided by common clock framework, not used + * + * Returns the current clock rate, or zero in failure. + */ +static long clk_scu_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + /* + * Assume we support all the requested rate and let the SCU firmware + * to handle the left work + */ + return rate; +} + +/* + * clk_scu_set_rate - Set rate for a SCU clock + * @hw: clock to change rate for + * @rate: target rate for the clock + * @parent_rate: rate of the clock parent, not used for SCU clocks + * + * Sets a clock frequency for a SCU clock. Returns the SCU + * protocol status. + */ +static int clk_scu_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct clk_scu *clk = to_clk_scu(hw); + struct imx_sc_msg_req_set_clock_rate msg; + struct imx_sc_rpc_msg *hdr = &msg.hdr; + + hdr->ver = IMX_SC_RPC_VERSION; + hdr->svc = IMX_SC_RPC_SVC_PM; + hdr->func = IMX_SC_PM_FUNC_SET_CLOCK_RATE; + hdr->size = 3; + + msg.rate = cpu_to_le32(rate); + msg.resource = cpu_to_le16(clk->rsrc_id); + msg.clk = clk->clk_type; + + return imx_scu_call_rpc(ccm_ipc_handle, &msg, true); +} + +static int sc_pm_clock_enable(struct imx_sc_ipc *ipc, u16 resource, + u8 clk, bool enable, bool autog) +{ + struct imx_sc_msg_req_clock_enable msg; + struct imx_sc_rpc_msg *hdr = &msg.hdr; + + hdr->ver = IMX_SC_RPC_VERSION; + hdr->svc = IMX_SC_RPC_SVC_PM; + hdr->func = IMX_SC_PM_FUNC_CLOCK_ENABLE; + hdr->size = 3; + + msg.resource = cpu_to_le16(resource); + msg.clk = clk; + msg.enable = enable; + msg.autog = autog; + + return imx_scu_call_rpc(ccm_ipc_handle, &msg, true); +} + +/* + * clk_scu_prepare - Enable a SCU clock + * @hw: clock to enable + * + * Enable the clock at the DSC slice level + */ +static int clk_scu_prepare(struct clk_hw *hw) +{ + struct clk_scu *clk = to_clk_scu(hw); + + return sc_pm_clock_enable(ccm_ipc_handle, clk->rsrc_id, + clk->clk_type, true, false); +} + +/* + * clk_scu_unprepare - Disable a SCU clock + * @hw: clock to enable + * + * Disable the clock at the DSC slice level + */ +static void clk_scu_unprepare(struct clk_hw *hw) +{ + struct clk_scu *clk = to_clk_scu(hw); + int ret; + + ret = sc_pm_clock_enable(ccm_ipc_handle, clk->rsrc_id, + clk->clk_type, false, false); + if (ret) + pr_warn("%s: clk unprepare failed %d\n", clk_hw_get_name(hw), + ret); +} + +static const struct clk_ops clk_scu_ops = { + .recalc_rate = clk_scu_recalc_rate, + .round_rate = clk_scu_round_rate, + .set_rate = clk_scu_set_rate, + .prepare = clk_scu_prepare, + .unprepare = clk_scu_unprepare, +}; + +struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id, u8 clk_type) +{ + struct clk_init_data init; + struct clk_scu *clk; + struct clk_hw *hw; + int ret; + + clk = kzalloc(sizeof(*clk), GFP_KERNEL); + if (!clk) + return ERR_PTR(-ENOMEM); + + clk->rsrc_id = rsrc_id; + clk->clk_type = clk_type; + + init.name = name; + init.ops = &clk_scu_ops; + init.num_parents = 0; + /* + * Note on MX8, the clocks are tightly coupled with power domain + * that once the power domain is off, the clock status may be + * lost. So we make it NOCACHE to let user to retrieve the real + * clock status from HW instead of using the possible invalid + * cached rate. + */ + init.flags = CLK_GET_RATE_NOCACHE; + clk->hw.init = &init; + + hw = &clk->hw; + ret = clk_hw_register(NULL, hw); + if (ret) { + kfree(clk); + hw = ERR_PTR(ret); + } + + return hw; +} diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h new file mode 100644 index 0000000..309b4de --- /dev/null +++ b/drivers/clk/imx/clk-scu.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright 2018 NXP + * Dong Aisheng <aisheng.dong@nxp.com> + */ + +#ifndef __IMX_CLK_SCU_H +#define __IMX_CLK_SCU_H + +#include <linux/firmware/imx/sci.h> + +int imx_clk_scu_init(void); +struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id, u8 clk_type); + +#endif
Add SCU clock common part which will be used by client clock drivers. SCU clocks are totally different from the legacy clocks (No much legacy things can be reused), it's using a firmware interface now based on SCU protocol. So a new configuration option CONFIG_MXC_CLK_SCU is added. Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Fabio Estevam <fabio.estevam@nxp.com> Cc: Stephen Boyd <sboyd@kernel.org> Cc: Michael Turquette <mturquette@baylibre.com> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- v10->v11: * break dependency on the arch Kconfig v9->v10: * remove ccm_ipc_handle from headfile as no other users need it now and move imx_clk_scu_init() implementation into driver accordingly. v8->v9: * improve commit message to detail the SCU clock difference from legecy clock * add __le32 and __le16 for SCU message structure members as it's little endian v7->v8: * add kernel doc for scu protocol structures v7: * no changes v5->v6: * simply the whole clock driver a lot by re-orgnizing the driver into a few clock types:: scu clock (merge scu divider/gate/mux) and scu gpr lock which accessing is through SCU protocol and LPCG clock which is directly accessible by CPU. * LPCG clock support will be added in next patch, gpr clock is still not used and will be added later. * remove old year license as the code is totally rewritten * scu mux support will be added later as it's also still not used. v4->v5: * add more explanation in commit message on why put scu clocks in a deeper folder. * move scu clk files into the top directory of imx folder v3->v4: * scu headfile path change v2->v3: * no changes v1->v2: * update function call name --- drivers/clk/imx/Kconfig | 6 ++ drivers/clk/imx/Makefile | 3 + drivers/clk/imx/clk-scu.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/clk/imx/clk-scu.h | 15 +++ 4 files changed, 294 insertions(+) create mode 100644 drivers/clk/imx/clk-scu.c create mode 100644 drivers/clk/imx/clk-scu.h