Re: [PATCH V8 2/7] clk: imx: add scu clock common part
From: Stephen Boyd <sboyd@kernel.org>
Date: 2018-12-03 18:33:49
Also in:
linux-clk
Quoting Aisheng DONG (2018-11-21 06:12:23)
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). So a new configuration option CONFIG_MXC_CLK_SCU is added.
Why are they different? Can you add more details on what has changed here? I hope to see something like "it's a firmware interface now".
quoted hunk ↗ jump to hunk
Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Fabio Estevam <redacted> Cc: Stephen Boyd <sboyd@kernel.org> Cc: Michael Turquette <mturquette@baylibre.com> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c new file mode 100644 index 0000000..5f9c1f6 --- /dev/null +++ b/drivers/clk/imx/clk-scu.c@@ -0,0 +1,267 @@ +// 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 + * + * This structure describes one SCU clock
Drop this sentence, it duplicates the first line of this kernel-doc.
+ */
+struct clk_scu {
+ struct clk_hw hw;
+ u32 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;
+ u32 rate;
+ u16 resource;These should be __le32 and __le16 presumably because the firmware interface we're talking to is little endian?
+ u8 clk;
+} __packed;
+
+struct req_get_clock_rate {
+ u16 resource;Same.
+ u8 clk;
+} __packed;
+
+struct resp_get_clock_rate {
+ u32 rate;Same.
+};
+
+/*
+ * 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;
+} __packed;Does this need __packed? struct imx_sc_rpc_msg is naturally aligned so I think we don't need this.
+
+/*
+ * 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;
+ u16 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);
+}
+
+/*
+ * 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;I really hope msg isn't DMAed to the firmware, because the response is in there!
+ 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 = 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 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 = rate;
+ msg.resource = clk->rsrc_id;
+ msg.clk = clk->clk_type;
+
+ return imx_scu_call_rpc(ccm_ipc_handle, &msg, true);This function returns normal error codes? Or some firmware special error code? Please make sure we return normal linux errno values from this driver.
quoted hunk ↗ jump to hunk
+} + +static int sc_pm_clock_enable(struct imx_sc_ipc *ipc, u32 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 = 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..09f381b --- /dev/null +++ b/drivers/clk/imx/clk-scu.h@@ -0,0 +1,21 @@ +/* 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> + +extern struct imx_sc_ipc *ccm_ipc_handle; + +static inline int imx_clk_scu_init(void)
Is this supposed to return EPROBE_DEFER or something? Who calls this?
+{
+ return imx_scu_get_handle(&ccm_ipc_handle);
+}
+
+struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id, u8 clk_type);
+_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel