| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <423a4c05-ccd8-4bcb-b686-c1eba79a6a3d@linaro.org>
Date: Tue, 3 Jan 2023 16:19:12 +0100
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
Andy Gross <agross@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] clk: qcom: add SM8550 DISPCC driver
On 03/01/2023 15:24, Dmitry Baryshkov wrote:
> On Tue, 3 Jan 2023 at 15:54, Neil Armstrong <neil.armstrong@...aro.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@...aro.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
>
>
>
Powered by blists - more mailing lists