| 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: <aNbXdFPrDr8V2a+1@lizhi-Precision-Tower-5810>
Date: Fri, 26 Sep 2025 14:12:04 -0400
From: Frank Li <Frank.li@....com>
To: Anand Moon <linux.amoon@...il.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kwilczynski@...nel.org>,
Manivannan Sadhasivam <mani@...nel.org>,
Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Thierry Reding <thierry.reding@...il.com>,
Jonathan Hunter <jonathanh@...dia.com>,
"open list:PCI SUBSYSTEM" <linux-pci@...r.kernel.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
"open list:TEGRA ARCHITECTURE SUPPORT" <linux-tegra@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 2/5] PCI: tegra: Simplify clock handling by using
clk_bulk*() functions
On Fri, Sep 26, 2025 at 12:57:43PM +0530, Anand Moon wrote:
> Currently, the driver acquires clocks and prepare/enable/disable/unprepare
> the clocks individually thereby making the driver complex to read.
>
> The driver can be simplified by using the clk_bulk*() APIs.
>
> Use:
> - devm_clk_bulk_get() API to acquire all the clocks
> - clk_bulk_prepare_enable() to prepare/enable clocks
> - clk_bulk_disable_unprepare() APIs to disable/unprepare them in bulk
>
> Following change also removes the legacy has_cml_clk flag and its associated
> conditional logic. Instead, the driver now relies on the clock definitions from
> the device tree to determine the correct clock sequencing.
> This reduces hardcoded dependencies and improves the driver's maintainability.
>
> Cc: Thierry Reding <thierry.reding@...il.com>
> Cc: Jon Hunter <jonathanh@...dia.com>
> Signed-off-by: Anand Moon <linux.amoon@...il.com>
> ---
> v1: Switch from devm_clk_bulk_get_all() -> devm_clk_bulk_get() with
> fix clks array.
why not use devm_clk_bulk_get_all()?
Frank
>
> nvidia,tegra20-pcie and nvidia,tegra186-pcie uses three clocks
> pex, afi, pll_e
> where as nvidia,tegra30-pcie, nvidia,tegra124-pcie, nvidia,tegra210-pcie
> uses four clks
> pex, afi, pll_e, cml
> ---
> ---
> drivers/pci/controller/pci-tegra.c | 100 +++++++++++++----------------
> 1 file changed, 45 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index 467ddc701adc..07a61d902eae 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -287,6 +287,8 @@ struct tegra_pcie_port_soc {
> struct tegra_pcie_soc {
> unsigned int num_ports;
> const struct tegra_pcie_port_soc *ports;
> + const char * const *clk_names;
> + unsigned int num_clks;
> unsigned int msi_base_shift;
> unsigned long afi_pex2_ctrl;
> u32 pads_pll_ctl;
> @@ -297,7 +299,6 @@ struct tegra_pcie_soc {
> bool has_pex_clkreq_en;
> bool has_pex_bias_ctrl;
> bool has_intr_prsnt_sense;
> - bool has_cml_clk;
> bool has_gen2;
> bool force_pca_enable;
> bool program_uphy;
> @@ -330,10 +331,7 @@ struct tegra_pcie {
>
> struct resource cs;
>
> - struct clk *pex_clk;
> - struct clk *afi_clk;
> - struct clk *pll_e;
> - struct clk *cml_clk;
> + struct clk_bulk_data *clks;
>
> struct reset_control *pex_rst;
> struct reset_control *afi_rst;
> @@ -1158,10 +1156,7 @@ static void tegra_pcie_power_off(struct tegra_pcie *pcie)
>
> reset_control_assert(pcie->afi_rst);
>
> - clk_disable_unprepare(pcie->pll_e);
> - if (soc->has_cml_clk)
> - clk_disable_unprepare(pcie->cml_clk);
> - clk_disable_unprepare(pcie->afi_clk);
> + clk_bulk_disable_unprepare(soc->num_clks, pcie->clks);
>
> if (!dev->pm_domain)
> tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> @@ -1202,35 +1197,16 @@ static int tegra_pcie_power_on(struct tegra_pcie *pcie)
> }
> }
>
> - err = clk_prepare_enable(pcie->afi_clk);
> + err = clk_bulk_prepare_enable(soc->num_clks, pcie->clks);
> if (err < 0) {
> - dev_err(dev, "failed to enable AFI clock: %d\n", err);
> + dev_err(dev, "filed to enable clocks: %d\n", err);
> goto powergate;
> }
>
> - if (soc->has_cml_clk) {
> - err = clk_prepare_enable(pcie->cml_clk);
> - if (err < 0) {
> - dev_err(dev, "failed to enable CML clock: %d\n", err);
> - goto disable_afi_clk;
> - }
> - }
> -
> - err = clk_prepare_enable(pcie->pll_e);
> - if (err < 0) {
> - dev_err(dev, "failed to enable PLLE clock: %d\n", err);
> - goto disable_cml_clk;
> - }
> -
> reset_control_deassert(pcie->afi_rst);
>
> return 0;
>
> -disable_cml_clk:
> - if (soc->has_cml_clk)
> - clk_disable_unprepare(pcie->cml_clk);
> -disable_afi_clk:
> - clk_disable_unprepare(pcie->afi_clk);
> powergate:
> if (!dev->pm_domain)
> tegra_powergate_power_off(TEGRA_POWERGATE_PCIE);
> @@ -1255,26 +1231,21 @@ static int tegra_pcie_clocks_get(struct tegra_pcie *pcie)
> {
> struct device *dev = pcie->dev;
> const struct tegra_pcie_soc *soc = pcie->soc;
> + int ret, i;
>
> - pcie->pex_clk = devm_clk_get(dev, "pex");
> - if (IS_ERR(pcie->pex_clk))
> - return PTR_ERR(pcie->pex_clk);
> -
> - pcie->afi_clk = devm_clk_get(dev, "afi");
> - if (IS_ERR(pcie->afi_clk))
> - return PTR_ERR(pcie->afi_clk);
> + pcie->clks = devm_kcalloc(dev, soc->num_clks, sizeof(*pcie->clks),
> + GFP_KERNEL);
> + if (!pcie->clks)
> + return -ENOMEM;
>
> - pcie->pll_e = devm_clk_get(dev, "pll_e");
> - if (IS_ERR(pcie->pll_e))
> - return PTR_ERR(pcie->pll_e);
> + for (i = 0; i < soc->num_clks; i++)
> + pcie->clks[i].id = soc->clk_names[i];
>
> - if (soc->has_cml_clk) {
> - pcie->cml_clk = devm_clk_get(dev, "cml");
> - if (IS_ERR(pcie->cml_clk))
> - return PTR_ERR(pcie->cml_clk);
> - }
> + ret = devm_clk_bulk_get(dev, soc->num_clks, pcie->clks);
> + if (ret)
> + dev_err(dev, "failed to get PCIe clocks: %d\n", ret);
>
> - return 0;
> + return ret;
> }
>
> static int tegra_pcie_resets_get(struct tegra_pcie *pcie)
> @@ -2335,9 +2306,17 @@ static const struct tegra_pcie_port_soc tegra20_pcie_ports[] = {
> { .pme.turnoff_bit = 8, .pme.ack_bit = 10 },
> };
>
> +static const char * const tegra20_pcie_clks[] = {
> + "pex",
> + "afi",
> + "pll_e",
> +};
> +
> static const struct tegra_pcie_soc tegra20_pcie = {
> .num_ports = 2,
> .ports = tegra20_pcie_ports,
> + .clk_names = tegra20_pcie_clks,
> + .num_clks = ARRAY_SIZE(tegra20_pcie_clks),
> .msi_base_shift = 0,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA20,
> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_DIV10,
> @@ -2345,7 +2324,6 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> .has_pex_clkreq_en = false,
> .has_pex_bias_ctrl = false,
> .has_intr_prsnt_sense = false,
> - .has_cml_clk = false,
> .has_gen2 = false,
> .force_pca_enable = false,
> .program_uphy = true,
> @@ -2356,6 +2334,13 @@ static const struct tegra_pcie_soc tegra20_pcie = {
> .ectl.enable = false,
> };
>
> +static const char * const tegra30_pcie_clks[] = {
> + "pex",
> + "afi",
> + "pll_e",
> + "cml",
> +};
> +
> static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = {
> { .pme.turnoff_bit = 0, .pme.ack_bit = 5 },
> { .pme.turnoff_bit = 8, .pme.ack_bit = 10 },
> @@ -2365,6 +2350,8 @@ static const struct tegra_pcie_port_soc tegra30_pcie_ports[] = {
> static const struct tegra_pcie_soc tegra30_pcie = {
> .num_ports = 3,
> .ports = tegra30_pcie_ports,
> + .clk_names = tegra30_pcie_clks,
> + .num_clks = ARRAY_SIZE(tegra30_pcie_clks),
> .msi_base_shift = 8,
> .afi_pex2_ctrl = 0x128,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> @@ -2374,7 +2361,6 @@ static const struct tegra_pcie_soc tegra30_pcie = {
> .has_pex_clkreq_en = true,
> .has_pex_bias_ctrl = true,
> .has_intr_prsnt_sense = true,
> - .has_cml_clk = true,
> .has_gen2 = false,
> .force_pca_enable = false,
> .program_uphy = true,
> @@ -2388,6 +2374,8 @@ static const struct tegra_pcie_soc tegra30_pcie = {
> static const struct tegra_pcie_soc tegra124_pcie = {
> .num_ports = 2,
> .ports = tegra20_pcie_ports,
> + .clk_names = tegra30_pcie_clks,
> + .num_clks = ARRAY_SIZE(tegra30_pcie_clks),
> .msi_base_shift = 8,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> @@ -2395,7 +2383,6 @@ static const struct tegra_pcie_soc tegra124_pcie = {
> .has_pex_clkreq_en = true,
> .has_pex_bias_ctrl = true,
> .has_intr_prsnt_sense = true,
> - .has_cml_clk = true,
> .has_gen2 = true,
> .force_pca_enable = false,
> .program_uphy = true,
> @@ -2409,6 +2396,8 @@ static const struct tegra_pcie_soc tegra124_pcie = {
> static const struct tegra_pcie_soc tegra210_pcie = {
> .num_ports = 2,
> .ports = tegra20_pcie_ports,
> + .clk_names = tegra30_pcie_clks,
> + .num_clks = ARRAY_SIZE(tegra30_pcie_clks),
> .msi_base_shift = 8,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> .tx_ref_sel = PADS_PLL_CTL_TXCLKREF_BUF_EN,
> @@ -2418,7 +2407,6 @@ static const struct tegra_pcie_soc tegra210_pcie = {
> .has_pex_clkreq_en = true,
> .has_pex_bias_ctrl = true,
> .has_intr_prsnt_sense = true,
> - .has_cml_clk = true,
> .has_gen2 = true,
> .force_pca_enable = true,
> .program_uphy = true,
> @@ -2450,6 +2438,8 @@ static const struct tegra_pcie_port_soc tegra186_pcie_ports[] = {
> static const struct tegra_pcie_soc tegra186_pcie = {
> .num_ports = 3,
> .ports = tegra186_pcie_ports,
> + .clk_names = tegra20_pcie_clks,
> + .num_clks = ARRAY_SIZE(tegra20_pcie_clks),
> .msi_base_shift = 8,
> .afi_pex2_ctrl = 0x19c,
> .pads_pll_ctl = PADS_PLL_CTL_TEGRA30,
> @@ -2459,7 +2449,6 @@ static const struct tegra_pcie_soc tegra186_pcie = {
> .has_pex_clkreq_en = true,
> .has_pex_bias_ctrl = true,
> .has_intr_prsnt_sense = true,
> - .has_cml_clk = false,
> .has_gen2 = true,
> .force_pca_enable = false,
> .program_uphy = false,
> @@ -2651,6 +2640,7 @@ static void tegra_pcie_remove(struct platform_device *pdev)
> static int tegra_pcie_pm_suspend(struct device *dev)
> {
> struct tegra_pcie *pcie = dev_get_drvdata(dev);
> + const struct tegra_pcie_soc *soc = pcie->soc;
> struct tegra_pcie_port *port;
> int err;
>
> @@ -2672,7 +2662,7 @@ static int tegra_pcie_pm_suspend(struct device *dev)
> }
>
> reset_control_assert(pcie->pex_rst);
> - clk_disable_unprepare(pcie->pex_clk);
> + clk_bulk_disable_unprepare(soc->num_clks, pcie->clks);
>
> if (IS_ENABLED(CONFIG_PCI_MSI))
> tegra_pcie_disable_msi(pcie);
> @@ -2686,6 +2676,7 @@ static int tegra_pcie_pm_suspend(struct device *dev)
> static int tegra_pcie_pm_resume(struct device *dev)
> {
> struct tegra_pcie *pcie = dev_get_drvdata(dev);
> + const struct tegra_pcie_soc *soc = pcie->soc;
> int err;
>
> err = tegra_pcie_power_on(pcie);
> @@ -2706,9 +2697,9 @@ static int tegra_pcie_pm_resume(struct device *dev)
> if (IS_ENABLED(CONFIG_PCI_MSI))
> tegra_pcie_enable_msi(pcie);
>
> - err = clk_prepare_enable(pcie->pex_clk);
> + err = clk_bulk_prepare_enable(soc->num_clks, pcie->clks);
> if (err) {
> - dev_err(dev, "failed to enable PEX clock: %d\n", err);
> + dev_err(dev, "failed to enable clock: %d\n", err);
> goto pex_dpd_enable;
> }
>
> @@ -2729,7 +2720,6 @@ static int tegra_pcie_pm_resume(struct device *dev)
>
> disable_pex_clk:
> reset_control_assert(pcie->pex_rst);
> - clk_disable_unprepare(pcie->pex_clk);
> pex_dpd_enable:
> pinctrl_pm_select_idle_state(dev);
> poweroff:
> --
> 2.50.1
>
Powered by blists - more mailing lists