From: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Bjorn Andersson <andersson@kernel.org>,
Andy Gross <agross@kernel.org>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver
Date: Tue, 3 Jan 2023 16:19:12 +0100 [thread overview]
Message-ID: <423a4c05-ccd8-4bcb-b686-c1eba79a6a3d@linaro.org> (raw)
In-Reply-To: <CAA8EJprLTLCskyTOLzfchNt1mrCUu47qMH43REOKbY0c3CxYTw@mail.gmail.com>
On 03/01/2023 15:24, Dmitry Baryshkov wrote:
> On Tue, 3 Jan 2023 at 15:54, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> Add support for the display clock controller found in SM8550
>> based devices.
>>
>> This clock controller feeds the Multimedia Display SubSystem (MDSS).
>> This driver is based on the SM8450 support.
>
> Looks good, few minor nits below:
>
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> drivers/clk/qcom/Kconfig | 9 +
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/dispcc-sm8550.c | 1814 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1824 insertions(+)
>>
>
> [skipped]
>
>> +static struct clk_regmap_div disp_cc_mdss_byte0_div_clk_src = {
>> + .reg = 0x8120,
>> + .shift = 0,
>> + .width = 4,
>> + .clkr.hw.init = &(struct clk_init_data) {
>> + .name = "disp_cc_mdss_byte0_div_clk_src",
>> + .parent_data = &(const struct clk_parent_data) {
>> + .hw = &disp_cc_mdss_byte0_clk_src.clkr.hw,
>> + },
>
> We can use parent_hws in such cases (here and below).
Sure, will switch to parent_hws for v2.
>
>> + .num_parents = 1,
>> + .ops = &clk_regmap_div_ops,
>> + },
>> +};
>> +
>> +static struct clk_regmap_div disp_cc_mdss_byte1_div_clk_src = {
>> + .reg = 0x813c,
>> + .shift = 0,
>> + .width = 4,
>> + .clkr.hw.init = &(struct clk_init_data) {
>> + .name = "disp_cc_mdss_byte1_div_clk_src",
>> + .parent_data = &(const struct clk_parent_data) {
>> + .hw = &disp_cc_mdss_byte1_clk_src.clkr.hw,
>> + },
>> + .num_parents = 1,
>> + .ops = &clk_regmap_div_ops,
>> + },
>> +};
>
> [skipped most of the clocks]
>
>> +static struct gdsc mdss_gdsc = {
>> + .gdscr = 0x9000,
>> + .pd = {
>> + .name = "mdss_gdsc",
>> + },
>> + .pwrsts = PWRSTS_OFF_ON,
>> + .flags = HW_CTRL | RETAIN_FF_ENABLE,
>> +};
>> +
>> +static struct gdsc mdss_int2_gdsc = {
>> + .gdscr = 0xb000,
>> + .pd = {
>> + .name = "mdss_int2_gdsc",
>> + },
>> + .pwrsts = PWRSTS_OFF_ON,
>> + .flags = HW_CTRL | RETAIN_FF_ENABLE,
>> +};
>> +
>> +static struct clk_regmap *disp_cc_sm8550_clocks[] = {
>> + [DISP_CC_MDSS_ACCU_CLK] = &disp_cc_mdss_accu_clk.clkr,
>> + [DISP_CC_MDSS_AHB1_CLK] = &disp_cc_mdss_ahb1_clk.clkr,
>> + [DISP_CC_MDSS_AHB_CLK] = &disp_cc_mdss_ahb_clk.clkr,
>> + [DISP_CC_MDSS_AHB_CLK_SRC] = &disp_cc_mdss_ahb_clk_src.clkr,
>> + [DISP_CC_MDSS_BYTE0_CLK] = &disp_cc_mdss_byte0_clk.clkr,
>> + [DISP_CC_MDSS_BYTE0_CLK_SRC] = &disp_cc_mdss_byte0_clk_src.clkr,
>> + [DISP_CC_MDSS_BYTE0_DIV_CLK_SRC] = &disp_cc_mdss_byte0_div_clk_src.clkr,
>> + [DISP_CC_MDSS_BYTE0_INTF_CLK] = &disp_cc_mdss_byte0_intf_clk.clkr,
>> + [DISP_CC_MDSS_BYTE1_CLK] = &disp_cc_mdss_byte1_clk.clkr,
>> + [DISP_CC_MDSS_BYTE1_CLK_SRC] = &disp_cc_mdss_byte1_clk_src.clkr,
>> + [DISP_CC_MDSS_BYTE1_DIV_CLK_SRC] = &disp_cc_mdss_byte1_div_clk_src.clkr,
>> + [DISP_CC_MDSS_BYTE1_INTF_CLK] = &disp_cc_mdss_byte1_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_AUX_CLK] = &disp_cc_mdss_dptx0_aux_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_AUX_CLK_SRC] = &disp_cc_mdss_dptx0_aux_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX0_CRYPTO_CLK] = &disp_cc_mdss_dptx0_crypto_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_LINK_CLK] = &disp_cc_mdss_dptx0_link_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_LINK_CLK_SRC] = &disp_cc_mdss_dptx0_link_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX0_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx0_link_div_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX0_LINK_INTF_CLK] = &disp_cc_mdss_dptx0_link_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_PIXEL0_CLK] = &disp_cc_mdss_dptx0_pixel0_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx0_pixel0_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX0_PIXEL1_CLK] = &disp_cc_mdss_dptx0_pixel1_clk.clkr,
>> + [DISP_CC_MDSS_DPTX0_PIXEL1_CLK_SRC] = &disp_cc_mdss_dptx0_pixel1_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX0_USB_ROUTER_LINK_INTF_CLK] =
>> + &disp_cc_mdss_dptx0_usb_router_link_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_AUX_CLK] = &disp_cc_mdss_dptx1_aux_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_AUX_CLK_SRC] = &disp_cc_mdss_dptx1_aux_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX1_CRYPTO_CLK] = &disp_cc_mdss_dptx1_crypto_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_LINK_CLK] = &disp_cc_mdss_dptx1_link_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_LINK_CLK_SRC] = &disp_cc_mdss_dptx1_link_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX1_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx1_link_div_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX1_LINK_INTF_CLK] = &disp_cc_mdss_dptx1_link_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_PIXEL0_CLK] = &disp_cc_mdss_dptx1_pixel0_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx1_pixel0_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX1_PIXEL1_CLK] = &disp_cc_mdss_dptx1_pixel1_clk.clkr,
>> + [DISP_CC_MDSS_DPTX1_PIXEL1_CLK_SRC] = &disp_cc_mdss_dptx1_pixel1_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX1_USB_ROUTER_LINK_INTF_CLK] =
>> + &disp_cc_mdss_dptx1_usb_router_link_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_AUX_CLK] = &disp_cc_mdss_dptx2_aux_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_AUX_CLK_SRC] = &disp_cc_mdss_dptx2_aux_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX2_CRYPTO_CLK] = &disp_cc_mdss_dptx2_crypto_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_LINK_CLK] = &disp_cc_mdss_dptx2_link_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_LINK_CLK_SRC] = &disp_cc_mdss_dptx2_link_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX2_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx2_link_div_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX2_LINK_INTF_CLK] = &disp_cc_mdss_dptx2_link_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_PIXEL0_CLK] = &disp_cc_mdss_dptx2_pixel0_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx2_pixel0_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX2_PIXEL1_CLK] = &disp_cc_mdss_dptx2_pixel1_clk.clkr,
>> + [DISP_CC_MDSS_DPTX2_PIXEL1_CLK_SRC] = &disp_cc_mdss_dptx2_pixel1_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX3_AUX_CLK] = &disp_cc_mdss_dptx3_aux_clk.clkr,
>> + [DISP_CC_MDSS_DPTX3_AUX_CLK_SRC] = &disp_cc_mdss_dptx3_aux_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX3_CRYPTO_CLK] = &disp_cc_mdss_dptx3_crypto_clk.clkr,
>> + [DISP_CC_MDSS_DPTX3_LINK_CLK] = &disp_cc_mdss_dptx3_link_clk.clkr,
>> + [DISP_CC_MDSS_DPTX3_LINK_CLK_SRC] = &disp_cc_mdss_dptx3_link_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX3_LINK_DIV_CLK_SRC] = &disp_cc_mdss_dptx3_link_div_clk_src.clkr,
>> + [DISP_CC_MDSS_DPTX3_LINK_INTF_CLK] = &disp_cc_mdss_dptx3_link_intf_clk.clkr,
>> + [DISP_CC_MDSS_DPTX3_PIXEL0_CLK] = &disp_cc_mdss_dptx3_pixel0_clk.clkr,
>> + [DISP_CC_MDSS_DPTX3_PIXEL0_CLK_SRC] = &disp_cc_mdss_dptx3_pixel0_clk_src.clkr,
>> + [DISP_CC_MDSS_ESC0_CLK] = &disp_cc_mdss_esc0_clk.clkr,
>> + [DISP_CC_MDSS_ESC0_CLK_SRC] = &disp_cc_mdss_esc0_clk_src.clkr,
>> + [DISP_CC_MDSS_ESC1_CLK] = &disp_cc_mdss_esc1_clk.clkr,
>> + [DISP_CC_MDSS_ESC1_CLK_SRC] = &disp_cc_mdss_esc1_clk_src.clkr,
>> + [DISP_CC_MDSS_MDP1_CLK] = &disp_cc_mdss_mdp1_clk.clkr,
>> + [DISP_CC_MDSS_MDP_CLK] = &disp_cc_mdss_mdp_clk.clkr,
>> + [DISP_CC_MDSS_MDP_CLK_SRC] = &disp_cc_mdss_mdp_clk_src.clkr,
>> + [DISP_CC_MDSS_MDP_LUT1_CLK] = &disp_cc_mdss_mdp_lut1_clk.clkr,
>> + [DISP_CC_MDSS_MDP_LUT_CLK] = &disp_cc_mdss_mdp_lut_clk.clkr,
>> + [DISP_CC_MDSS_NON_GDSC_AHB_CLK] = &disp_cc_mdss_non_gdsc_ahb_clk.clkr,
>> + [DISP_CC_MDSS_PCLK0_CLK] = &disp_cc_mdss_pclk0_clk.clkr,
>> + [DISP_CC_MDSS_PCLK0_CLK_SRC] = &disp_cc_mdss_pclk0_clk_src.clkr,
>> + [DISP_CC_MDSS_PCLK1_CLK] = &disp_cc_mdss_pclk1_clk.clkr,
>> + [DISP_CC_MDSS_PCLK1_CLK_SRC] = &disp_cc_mdss_pclk1_clk_src.clkr,
>> + [DISP_CC_MDSS_RSCC_AHB_CLK] = &disp_cc_mdss_rscc_ahb_clk.clkr,
>> + [DISP_CC_MDSS_RSCC_VSYNC_CLK] = &disp_cc_mdss_rscc_vsync_clk.clkr,
>> + [DISP_CC_MDSS_VSYNC1_CLK] = &disp_cc_mdss_vsync1_clk.clkr,
>> + [DISP_CC_MDSS_VSYNC_CLK] = &disp_cc_mdss_vsync_clk.clkr,
>> + [DISP_CC_MDSS_VSYNC_CLK_SRC] = &disp_cc_mdss_vsync_clk_src.clkr,
>> + [DISP_CC_PLL0] = &disp_cc_pll0.clkr,
>> + [DISP_CC_PLL1] = &disp_cc_pll1.clkr,
>> + [DISP_CC_SLEEP_CLK] = &disp_cc_sleep_clk.clkr,
>> + [DISP_CC_SLEEP_CLK_SRC] = &disp_cc_sleep_clk_src.clkr,
>> + [DISP_CC_XO_CLK_SRC] = &disp_cc_xo_clk_src.clkr,
>> +};
>> +
>> +static const struct qcom_reset_map disp_cc_sm8550_resets[] = {
>> + [DISP_CC_MDSS_CORE_BCR] = { 0x8000 },
>> + [DISP_CC_MDSS_CORE_INT2_BCR] = { 0xa000 },
>> + [DISP_CC_MDSS_RSCC_BCR] = { 0xc000 },
>> +};
>> +
>> +static struct gdsc *disp_cc_sm8550_gdscs[] = {
>> + [MDSS_GDSC] = &mdss_gdsc,
>> + [MDSS_INT2_GDSC] = &mdss_int2_gdsc,
>> +};
>> +
>> +static const struct regmap_config disp_cc_sm8550_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = 0x11008,
>> + .fast_io = true,
>> +};
>> +
>> +static struct qcom_cc_desc disp_cc_sm8550_desc = {
>> + .config = &disp_cc_sm8550_regmap_config,
>> + .clks = disp_cc_sm8550_clocks,
>> + .num_clks = ARRAY_SIZE(disp_cc_sm8550_clocks),
>> + .resets = disp_cc_sm8550_resets,
>> + .num_resets = ARRAY_SIZE(disp_cc_sm8550_resets),
>> + .gdscs = disp_cc_sm8550_gdscs,
>> + .num_gdscs = ARRAY_SIZE(disp_cc_sm8550_gdscs),
>> +};
>> +
>> +static const struct of_device_id disp_cc_sm8550_match_table[] = {
>> + { .compatible = "qcom,sm8550-dispcc" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, disp_cc_sm8550_match_table);
>> +
>> +static void disp_cc_sm8550_pm_runtime_disable(void *data)
>> +{
>> + pm_runtime_disable(data);
>> +}
>> +
>> +static int disp_cc_sm8550_probe(struct platform_device *pdev)
>> +{
>> + struct regmap *regmap;
>> + int ret;
>> +
>> + pm_runtime_enable(&pdev->dev);
>
> We can use devm_pm_runtime_enable() here.
Ack
Thanks,
Neil
>
>> +
>> + ret = devm_add_action_or_reset(&pdev->dev, disp_cc_sm8550_pm_runtime_disable, &pdev->dev);
>> + if (ret)
>> + return ret;
>> +
>> + ret = pm_runtime_resume_and_get(&pdev->dev);
>> + if (ret)
>> + return ret;
>> +
>> + regmap = qcom_cc_map(pdev, &disp_cc_sm8550_desc);
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + clk_lucid_evo_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
>> + clk_lucid_evo_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
>> +
>> + /* Enable clock gating for MDP clocks */
>> + regmap_update_bits(regmap, DISP_CC_MISC_CMD, 0x10, 0x10);
>> +
>> + /*
>> + * Keep clocks always enabled:
>> + * disp_cc_xo_clk
>> + */
>> + regmap_update_bits(regmap, 0xe054, BIT(0), BIT(0));
>> +
>> + ret = qcom_cc_really_probe(pdev, &disp_cc_sm8550_desc, regmap);
>> +
>> + pm_runtime_put(&pdev->dev);
>> +
>> + return ret;
>> +}
>> +
>> +static struct platform_driver disp_cc_sm8550_driver = {
>> + .probe = disp_cc_sm8550_probe,
>> + .driver = {
>> + .name = "disp_cc-sm8550",
>> + .of_match_table = disp_cc_sm8550_match_table,
>> + },
>> +};
>> +
>> +static int __init disp_cc_sm8550_init(void)
>> +{
>> + return platform_driver_register(&disp_cc_sm8550_driver);
>> +}
>> +subsys_initcall(disp_cc_sm8550_init);
>> +
>> +static void __exit disp_cc_sm8550_exit(void)
>> +{
>> + platform_driver_unregister(&disp_cc_sm8550_driver);
>> +}
>> +module_exit(disp_cc_sm8550_exit);
>> +
>> +MODULE_DESCRIPTION("QTI DISPCC SM8550 Driver");
>> +MODULE_LICENSE("GPL");
>>
>> --
>> 2.34.1
>
>
>
prev parent reply other threads:[~2023-01-03 15:19 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-03 13:53 [PATCH 0/3] clk: qcom: Add DISPCC driver for SM8550 Neil Armstrong
2023-01-03 13:53 ` [PATCH 1/3] dt-bindings: clock: document SM8550 DISPCC clock controller Neil Armstrong
2023-01-06 9:31 ` Krzysztof Kozlowski
2023-01-03 13:53 ` [PATCH 2/3] clk: qcom: clk-alpha-pll: define alias of LUCID OLE reset ops to EVO reset ops Neil Armstrong
2023-01-03 13:56 ` Konrad Dybcio
2023-01-03 13:54 ` [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver Neil Armstrong
2023-01-03 14:01 ` Konrad Dybcio
2023-01-03 14:24 ` Dmitry Baryshkov
2023-01-03 15:19 ` Neil Armstrong [this message]
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=423a4c05-ccd8-4bcb-b686-c1eba79a6a3d@linaro.org \
--to=neil.armstrong@linaro.org \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox