linux-kernel - Re: [PATCH v3 3/3] clk: qcom: add clock controller driver for qca8386/qca8084

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]

Date:   Thu, 10 Aug 2023 14:59:24 +0200
From:   Konrad Dybcio <konrad.dybcio@...aro.org>
To:     Luo Jie <quic_luoj@...cinc.com>, andersson@...nel.org,
        agross@...nel.org, mturquette@...libre.com, sboyd@...nel.org,
        robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
        conor+dt@...nel.org, catalin.marinas@....com, will@...nel.org,
        p.zabel@...gutronix.de
Cc:     linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        quic_srichara@...cinc.com
Subject: Re: [PATCH v3 3/3] clk: qcom: add clock controller driver for
 qca8386/qca8084

On 10.08.2023 13:54, Luo Jie wrote:
> Add clock & reset controller driver for qca8386/qca8084.
> 
> Signed-off-by: Luo Jie <quic_luoj@...cinc.com>
> ---

> +struct qcom_cc {
> +	struct qcom_reset_controller reset;
> +	struct clk_regmap **rclks;
> +	size_t num_rclks;
> +};
This all, including the probe func, is required because of the MDIO dance,
I assume?

Commonizing that would make more sense should more clocks like this appear
in the future.

[...]

> +static struct clk_branch nss_cc_switch_core_clk = {
> +	.halt_reg = 0x8,
> +	.clkr = {
> +		.enable_reg = 0x8,
> +		.enable_mask = BIT(0),
> +		.hw.init = &(const struct clk_init_data) {
> +			.name = "nss_cc_switch_core_clk",
> +			.parent_hws = (const struct clk_hw *[]) {
> +				&nss_cc_switch_core_clk_src.clkr.hw,
> +			},
> +			.num_parents = 1,
> +			/* Can be disabled in PHY mode for power saving */
Well it clearly cannot be disabled if it has the CLK_IS_CRITICAL flag :D

What's the "PHY mode" you're talking about?

> +			.flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL,
> +			.ops = &clk_branch2_mdio_ops,
> +		},
> +	},
> +};
I see a whole bunch of CRITICAL clocks.. please make sure these clocks
are actually necessary for Linux to know about (i.e. if we don't need
to call any operations on them, we might just skip registering them
with the driver).

Konrad