Re: [PATCH v3 3/5] clk: qcom: Add QDU1000 and QRU1000 GCC support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Oct 26, 2022 at 12:04:39PM -0700, Melody Olvera wrote:
> diff --git a/drivers/clk/qcom/gcc-qdu1000.c b/drivers/clk/qcom/gcc-qdu1000.c
> new file mode 100644
> index 000000000000..7bd8ebf0ddb5
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-qdu1000.c
> @@ -0,0 +1,2645 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,gcc-qdu1000.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "clk-regmap-divider.h"
> +#include "clk-regmap-mux.h"
> +#include "clk-regmap-phy-mux.h"
> +#include "reset.h"
> +
> +enum {
> +	P_BI_TCXO,
> +	P_GCC_GPLL0_OUT_EVEN,
> +	P_GCC_GPLL0_OUT_MAIN,
> +	P_GCC_GPLL1_OUT_MAIN,
> +	P_GCC_GPLL2_OUT_MAIN,
> +	P_GCC_GPLL3_OUT_MAIN,
> +	P_GCC_GPLL4_OUT_MAIN,
> +	P_GCC_GPLL5_OUT_MAIN,
> +	P_GCC_GPLL6_OUT_MAIN,
> +	P_GCC_GPLL7_OUT_MAIN,
> +	P_GCC_GPLL8_OUT_MAIN,
> +	P_PCIE_0_PHY_AUX_CLK,
> +	P_PCIE_0_PIPE_CLK,
> +	P_SLEEP_CLK,
> +	P_USB3_PHY_WRAPPER_GCC_USB30_PIPE_CLK,
> +};
[..]
> +static const struct clk_parent_data gcc_parent_data_1[] = {
> +	{ .index = P_BI_TCXO },
> +	{ .hw = &gcc_gpll0.clkr.hw },
> +	{ .index = P_SLEEP_CLK },

.index here refers to the index in the clocks property in DT.

I think it's okay to reuse the parent-enum, but the entries within must
then match the order defined in the DT binding. So you need to ensure
that the first N entires in the enum matches the binding.

Perhaps it's cleaner to just carry a separate enum for the clocks order,
as we've done in the other drivers?

If nothing else it makes it clear that one number space is arbitrary and
internal to the driver and the other is ABI.

> +	{ .hw = &gcc_gpll0_out_even.clkr.hw },
> +};
> +
[..]
> +static struct clk_regmap_mux gcc_pcie_0_phy_aux_clk_src = {
> +	.reg = 0x9d080,
> +	.shift = 0,
> +	.width = 2,
> +	.parent_map = gcc_parent_map_6,
> +	.clkr = {
> +		.hw.init = &(const struct clk_init_data){

Sorry for being picky, but I do like when there's a space between the
')' and '{' in these lines...

> +			.name = "gcc_pcie_0_phy_aux_clk_src",
> +			.parent_data = gcc_parent_data_6,
> +			.num_parents = ARRAY_SIZE(gcc_parent_data_6),
> +			.ops = &clk_regmap_mux_closest_ops,
> +		},
> +	},
> +};

Regards,
Bjorn



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux