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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA8EJprLTLCskyTOLzfchNt1mrCUu47qMH43REOKbY0c3CxYTw@mail.gmail.com>
Date:   Tue, 3 Jan 2023 16:24:11 +0200
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Neil Armstrong <neil.armstrong@...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 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).

> +               .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.

> +
> +       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



-- 
With best wishes
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ