linux-kernel - Re: [PATCH v2 3/3] clk: qcom: gcc-sdm845: add sdm670 global clock data

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: <8233a19a-9b0b-6962-6f36-381c8f380ad3@linaro.org>
Date:   Thu, 8 Sep 2022 13:10:08 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Richard Acayan <mailingradian@...il.com>,
        linux-arm-msm@...r.kernel.org
Cc:     andersson@...nel.org, agross@...nel.org,
        konrad.dybcio@...ainline.org, mturquette@...libre.com,
        sboyd@...nel.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, tdas@...eaurora.org,
        linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        ~postmarketos/upstreaming@...ts.sr.ht, caleb@...nolly.tech,
        jo@...amily.in
Subject: Re: [PATCH v2 3/3] clk: qcom: gcc-sdm845: add sdm670 global clock
 data

On 08/09/2022 00:39, Richard Acayan wrote:
> The Snapdragon 670 adds and removes some clocks, adds new frequencies, and
> adds a new GPLL (Global Phase-Locked Loop) in reference to SDM845, while
> also removing some GDSCs. Despite these differences, there are many
> similarities with SDM670. Add data for SDM670 in the driver for SDM845 to
> reuse the most of the clock data.
> 
> Advantages and disadvantages of this approach:
>  + maintenance applies to both sdm670 and sdm845 by default
>  + less duplicate code (clocks) means smaller distro/pre-built kernels
>    with all drivers enabled
>  - clocks for both SoC's must be compiled if the user wants clocks for one
>    specific SoC (both or none)
>  - additional testing needed for sdm845 devices
> 
> Link: https://android.googlesource.com/kernel/msm/+/443bd8d6e2cf54698234c752e6de97b4b8a528bd^!/#F10
> Signed-off-by: Richard Acayan <mailingradian@...il.com>
> ---
>  drivers/clk/qcom/Kconfig      |   4 +-
>  drivers/clk/qcom/gcc-sdm845.c | 398 ++++++++++++++++++++++++++++++++++
>  2 files changed, 400 insertions(+), 2 deletions(-)
> 

Thank you for your patch. There is something to discuss/improve.

> +					&gcc_tsif_inactivity_timers_clk.clkr,
> +	[GCC_TSIF_REF_CLK] = &gcc_tsif_ref_clk.clkr,
> +	[GCC_TSIF_REF_CLK_SRC] = &gcc_tsif_ref_clk_src.clkr,
> +	[GCC_UFS_MEM_CLKREF_CLK] = &gcc_ufs_mem_clkref_clk.clkr,
> +	[GCC_UFS_PHY_AHB_CLK] = &gcc_ufs_phy_ahb_clk.clkr,
> +	[GCC_UFS_PHY_AXI_CLK] = &gcc_ufs_phy_axi_clk.clkr,
> +	[GCC_UFS_PHY_AXI_CLK_SRC] = &gcc_ufs_phy_axi_clk_src.clkr,
> +	[GCC_UFS_PHY_ICE_CORE_CLK] = &gcc_ufs_phy_ice_core_clk.clkr,
> +	[GCC_UFS_PHY_ICE_CORE_CLK_SRC] = &gcc_ufs_phy_ice_core_clk_src.clkr,
> +	[GCC_UFS_PHY_PHY_AUX_CLK] = &gcc_ufs_phy_phy_aux_clk.clkr,
> +	[GCC_UFS_PHY_PHY_AUX_CLK_SRC] = &gcc_ufs_phy_phy_aux_clk_src.clkr,
> +	[GCC_UFS_PHY_RX_SYMBOL_0_CLK] = &gcc_ufs_phy_rx_symbol_0_clk.clkr,
> +	[GCC_UFS_PHY_TX_SYMBOL_0_CLK] = &gcc_ufs_phy_tx_symbol_0_clk.clkr,
> +	[GCC_UFS_PHY_UNIPRO_CORE_CLK] = &gcc_ufs_phy_unipro_core_clk.clkr,
> +	[GCC_UFS_PHY_UNIPRO_CORE_CLK_SRC] =
> +					&gcc_ufs_phy_unipro_core_clk_src.clkr,
> +	[GCC_USB30_PRIM_MASTER_CLK] = &gcc_usb30_prim_master_clk.clkr,
> +	[GCC_USB30_PRIM_MASTER_CLK_SRC] = &gcc_usb30_prim_master_clk_src.clkr,
> +	[GCC_USB30_PRIM_MOCK_UTMI_CLK] = &gcc_usb30_prim_mock_utmi_clk.clkr,
> +	[GCC_USB30_PRIM_MOCK_UTMI_CLK_SRC] =
> +					&gcc_usb30_prim_mock_utmi_clk_src.clkr,
> +	[GCC_USB30_PRIM_SLEEP_CLK] = &gcc_usb30_prim_sleep_clk.clkr,
> +	[GCC_USB3_PRIM_CLKREF_CLK] = &gcc_usb3_prim_clkref_clk.clkr,
> +	[GCC_USB3_PRIM_PHY_AUX_CLK] = &gcc_usb3_prim_phy_aux_clk.clkr,
> +	[GCC_USB3_PRIM_PHY_AUX_CLK_SRC] = &gcc_usb3_prim_phy_aux_clk_src.clkr,
> +	[GCC_USB3_PRIM_PHY_COM_AUX_CLK] = &gcc_usb3_prim_phy_com_aux_clk.clkr,
> +	[GCC_USB3_PRIM_PHY_PIPE_CLK] = &gcc_usb3_prim_phy_pipe_clk.clkr,
> +	[GCC_USB_PHY_CFG_AHB2PHY_CLK] = &gcc_usb_phy_cfg_ahb2phy_clk.clkr,
> +	[GCC_VDDA_VS_CLK] = &gcc_vdda_vs_clk.clkr,
> +	[GCC_VDDCX_VS_CLK] = &gcc_vddcx_vs_clk.clkr,
> +	[GCC_VDDMX_VS_CLK] = &gcc_vddmx_vs_clk.clkr,
> +	[GCC_VIDEO_AHB_CLK] = &gcc_video_ahb_clk.clkr,
> +	[GCC_VIDEO_AXI_CLK] = &gcc_video_axi_clk.clkr,
> +	[GCC_VIDEO_XO_CLK] = &gcc_video_xo_clk.clkr,
> +	[GCC_VS_CTRL_AHB_CLK] = &gcc_vs_ctrl_ahb_clk.clkr,
> +	[GCC_VS_CTRL_CLK] = &gcc_vs_ctrl_clk.clkr,
> +	[GCC_VS_CTRL_CLK_SRC] = &gcc_vs_ctrl_clk_src.clkr,
> +	[GCC_VSENSOR_CLK_SRC] = &gcc_vsensor_clk_src.clkr,
> +	[GPLL0] = &gpll0.clkr,
> +	[GPLL0_OUT_EVEN] = &gpll0_out_even.clkr,
> +	[GPLL4] = &gpll4.clkr,
> +	[GPLL6] = &gpll6.clkr,
> +	[GCC_CPUSS_DVM_BUS_CLK] = &gcc_cpuss_dvm_bus_clk.clkr,
> +	[GCC_CPUSS_GNOC_CLK] = &gcc_cpuss_gnoc_clk.clkr,
> +	[GCC_QSPI_CORE_CLK_SRC] = &gcc_qspi_core_clk_src.clkr,
> +	[GCC_QSPI_CORE_CLK] = &gcc_qspi_core_clk.clkr,
> +	[GCC_QSPI_CNOC_PERIPH_AHB_CLK] = &gcc_qspi_cnoc_periph_ahb_clk.clkr,
> +};
> +
>  static struct clk_regmap *gcc_sdm845_clocks[] = {
>  	[GCC_AGGRE_NOC_PCIE_TBU_CLK] = &gcc_aggre_noc_pcie_tbu_clk.clkr,
>  	[GCC_AGGRE_UFS_CARD_AXI_CLK] = &gcc_aggre_ufs_card_axi_clk.clkr,
> @@ -3515,6 +3881,7 @@ static const struct qcom_reset_map gcc_sdm845_resets[] = {
>  	[GCC_QUPV3_WRAPPER_1_BCR] = { 0x18000 },
>  	[GCC_QUSB2PHY_PRIM_BCR] = { 0x12000 },
>  	[GCC_QUSB2PHY_SEC_BCR] = { 0x12004 },
> +	[GCC_SDCC1_BCR] = { 0x26000 },

You are changing existing SDM845, so this should be separate patch with
its own explanation.


Best regards,
Krzysztof