Messages in this thread |  | | Subject | Re: [PATCH v2 5/5] clk: qcom: add video clock controller driver for SM8250 | From | Jonathan Marek <> | Date | Wed, 23 Sep 2020 12:07:16 -0400 |
| |
On 9/22/20 2:46 PM, Stephen Boyd wrote: > Quoting Jonathan Marek (2020-09-03 20:09:54) >> Add support for the video clock controller found on SM8250 based devices. >> >> Derived from the downstream driver. >> >> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >> --- >> drivers/clk/qcom/Kconfig | 9 + >> drivers/clk/qcom/Makefile | 1 + >> drivers/clk/qcom/videocc-sm8250.c | 518 ++++++++++++++++++++++++++++++ >> 3 files changed, 528 insertions(+) >> create mode 100644 drivers/clk/qcom/videocc-sm8250.c >> >> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig >> index 40d7ee9886c9..95efa38211d5 100644 >> --- a/drivers/clk/qcom/Kconfig >> +++ b/drivers/clk/qcom/Kconfig >> @@ -453,6 +453,15 @@ config SM_VIDEOCC_8150 >> Say Y if you want to support video devices and functionality such as >> video encode and decode. >> >> +config SM_VIDEOCC_8250 >> + tristate "SM8250 Video Clock Controller" >> + select SDM_GCC_8250 >> + select QCOM_GDSC >> + help >> + Support for the video clock controller on SM8250 devices. >> + Say Y if you want to support video devices and functionality such as >> + video encode and decode. >> + >> config SPMI_PMIC_CLKDIV >> tristate "SPMI PMIC clkdiv Support" >> depends on SPMI || COMPILE_TEST >> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile >> index 6f4c580d2728..55fb20800b66 100644 >> --- a/drivers/clk/qcom/Makefile >> +++ b/drivers/clk/qcom/Makefile >> @@ -69,6 +69,7 @@ obj-$(CONFIG_SM_GCC_8250) += gcc-sm8250.o >> obj-$(CONFIG_SM_GPUCC_8150) += gpucc-sm8150.o >> obj-$(CONFIG_SM_GPUCC_8250) += gpucc-sm8250.o >> obj-$(CONFIG_SM_VIDEOCC_8150) += videocc-sm8150.o >> +obj-$(CONFIG_SM_VIDEOCC_8250) += videocc-sm8250.o >> obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o >> obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o >> obj-$(CONFIG_QCOM_HFPLL) += hfpll.o >> diff --git a/drivers/clk/qcom/videocc-sm8250.c b/drivers/clk/qcom/videocc-sm8250.c >> new file mode 100644 >> index 000000000000..a814d10945c4 >> --- /dev/null >> +++ b/drivers/clk/qcom/videocc-sm8250.c >> @@ -0,0 +1,518 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved. >> + */ >> + >> +#include <linux/clk-provider.h> >> +#include <linux/module.h> >> +#include <linux/platform_device.h> >> +#include <linux/regmap.h> >> + >> +#include <dt-bindings/clock/qcom,videocc-sm8250.h> >> + > [...] >> +static struct clk_rcg2 video_cc_ahb_clk_src = { >> + .cmd_rcgr = 0xbd4, >> + .mnd_width = 0, >> + .hid_width = 5, >> + .parent_map = video_cc_parent_map_0, >> + .freq_tbl = ftbl_video_cc_ahb_clk_src, >> + .clkr.hw.init = &(struct clk_init_data){ >> + .name = "video_cc_ahb_clk_src", >> + .parent_data = video_cc_parent_data_0, >> + .num_parents = ARRAY_SIZE(video_cc_parent_data_0), >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_rcg2_shared_ops, >> + }, >> +}; >> + >> +static struct clk_rcg2 video_cc_xo_clk_src = { >> + .cmd_rcgr = 0xecc, >> + .mnd_width = 0, >> + .hid_width = 5, >> + .parent_map = video_cc_parent_map_0, >> + .freq_tbl = ftbl_video_cc_ahb_clk_src, >> + .clkr.hw.init = &(struct clk_init_data){ >> + .name = "video_cc_xo_clk_src", >> + .parent_data = video_cc_parent_data_0, >> + .num_parents = ARRAY_SIZE(video_cc_parent_data_0), >> + .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT, > > Similar critical clk comment, see below. > >> + .ops = &clk_rcg2_ops, >> + }, >> +}; >> + >> +static struct clk_branch video_cc_ahb_clk = { >> + .halt_reg = 0xe58, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0xe58, >> + .enable_mask = BIT(0), >> + .hw.init = &(struct clk_init_data){ >> + .name = "video_cc_ahb_clk", >> + .parent_data = &(const struct clk_parent_data){ >> + .hw = &video_cc_ahb_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT, > > Similar critical clk comment, see below. > >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + >> +static struct clk_branch video_cc_mvs0_clk = { >> + .halt_reg = 0xd34, >> + .halt_check = BRANCH_HALT_SKIP, /* TODO: hw gated ? */ > > Is this resolved? >
Downstream has this clock as BRANCH_HALT_VOTED, but with the upstream venus driver (with patches to enable sm8250), that results in a "video_cc_mvs0_clk status stuck at 'off" error. AFAIK venus enables/disables this clock on its own (venus still works without touching this clock), but I didn't want to remove this in case it might be needed. I removed these clocks in the v3 I just sent.
>> + .clkr = { >> + .enable_reg = 0xd34, >> + .enable_mask = BIT(0), >> + .hw.init = &(struct clk_init_data){ >> + .name = "video_cc_mvs0_clk", >> + .parent_data = &(const struct clk_parent_data){ >> + .hw = &video_cc_mvs0_div_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_SET_RATE_PARENT, >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> + > [...] >> + >> +static struct clk_branch video_cc_xo_clk = { >> + .halt_reg = 0xeec, >> + .halt_check = BRANCH_HALT, >> + .clkr = { >> + .enable_reg = 0xeec, >> + .enable_mask = BIT(0), >> + .hw.init = &(struct clk_init_data){ >> + .name = "video_cc_xo_clk", >> + .parent_data = &(const struct clk_parent_data){ >> + .hw = &video_cc_xo_clk_src.clkr.hw, >> + }, >> + .num_parents = 1, >> + .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT, > > Please add a coment why it's critical. If no consumer of this clk exists > it's preferred to move the enabling to probe and not waste memory > modeling it in software. > >> + .ops = &clk_branch2_ops, >> + }, >> + }, >> +}; >> +
|  |