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] [day] [month] [year] [list]
Message-ID: <1j34hqai39.fsf@starbuckisacylon.baylibre.com>
Date: Fri, 10 Jan 2025 14:55:54 +0100
From: Jerome Brunet <jbrunet@...libre.com>
To: Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,  Stephen Boyd
 <sboyd@...nel.org>,  Neil Armstrong <neil.armstrong@...aro.org>,  Kevin
 Hilman <khilman@...libre.com>,  Martin Blumenstingl
 <martin.blumenstingl@...glemail.com>,  chuan.liu@...ogic.com,
  linux-clk@...r.kernel.org,  linux-kernel@...r.kernel.org,
  linux-amlogic@...ts.infradead.org,  linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] clk: amlogic: c3: Limit the rate boundaries of clk_hw

On Fri 10 Jan 2025 at 19:47, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@...nel.org> wrote:

> From: Chuan Liu <chuan.liu@...ogic.com>
>
> The PLL can only stably lock within a limited frequency range.
>
> Due to timing constraints, the maximum frequency of the peripheral clock
> cannot exceed the design specifications.
>
> Signed-off-by: Chuan Liu <chuan.liu@...ogic.com>
> ---
>  drivers/clk/meson/c3-peripherals.c | 21 +++++++++++++++++++++
>  drivers/clk/meson/c3-pll.c         |  4 ++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
> index 7dcbf4ebee07..9f0a3990f0d6 100644
> --- a/drivers/clk/meson/c3-peripherals.c
> +++ b/drivers/clk/meson/c3-peripherals.c
> @@ -568,6 +568,7 @@ static const struct clk_parent_data pwm_parent_data[] = {
>  		.ops = &clk_regmap_gate_ops,			\
>  		.parent_names = (const char *[]) { #_name "_div" },\
>  		.num_parents = 1,				\
> +		.max_rate = 200000000,				\
>  		.flags = CLK_SET_RATE_PARENT,			\
>  	},							\
>  }
> @@ -724,6 +725,7 @@ static struct clk_regmap spicc_a = {
>  			&spicc_a_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 500000000,

I'm sorry but the whole thing is completly wrong.

All the clocks I'm seeing here are gates. This type of HW hardly cares
what rates it handles. Same goes from mux, dividers, etc ...

All you are doing here is trying enforce some made up "safety" / use-case
defined limits that do not belong in the clock controller.

The only piece of HW where limits could possibly make sense are PLL DCO,
and even there, you've got multiplier range which is way better as an
abstraction.

So it's a nack on the series.

If devices are have particular requirement on rate range, have the
related driver set it.


>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -771,6 +773,7 @@ static struct clk_regmap spicc_b = {
>  			&spicc_b_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 500000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -829,6 +832,7 @@ static struct clk_regmap spifc = {
>  			&spifc_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 167000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -887,6 +891,7 @@ static struct clk_regmap sd_emmc_a = {
>  			&sd_emmc_a_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 250000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -934,6 +939,7 @@ static struct clk_regmap sd_emmc_b = {
>  			&sd_emmc_b_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 250000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -981,6 +987,7 @@ static struct clk_regmap sd_emmc_c = {
>  			&sd_emmc_c_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 1200000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1074,6 +1081,7 @@ static struct clk_regmap eth_rmii = {
>  			&eth_rmii_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 50000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1132,6 +1140,7 @@ static struct clk_regmap mipi_dsi_meas = {
>  			&mipi_dsi_meas_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 200000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1190,6 +1199,7 @@ static struct clk_regmap dsi_phy = {
>  			&dsi_phy_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 1500000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1248,6 +1258,7 @@ static struct clk_regmap vout_mclk = {
>  			&vout_mclk_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 334000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1306,6 +1317,7 @@ static struct clk_regmap vout_enc = {
>  			&vout_enc_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 200000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1431,6 +1443,7 @@ static struct clk_regmap hcodec = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_data = hcodec_parent_data,
>  		.num_parents = ARRAY_SIZE(hcodec_parent_data),
> +		.max_rate = 667000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1489,6 +1502,7 @@ static struct clk_regmap vc9000e_aclk = {
>  			&vc9000e_aclk_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 667000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1536,6 +1550,7 @@ static struct clk_regmap vc9000e_core = {
>  			&vc9000e_core_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 400000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1594,6 +1609,7 @@ static struct clk_regmap csi_phy0 = {
>  			&csi_phy0_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 200000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1652,6 +1668,7 @@ static struct clk_regmap dewarpa = {
>  			&dewarpa_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 800000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1710,6 +1727,7 @@ static struct clk_regmap isp0 = {
>  			&isp0_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 400000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1768,6 +1786,7 @@ static struct clk_regmap nna_core = {
>  			&nna_core_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 800000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1826,6 +1845,7 @@ static struct clk_regmap ge2d = {
>  			&ge2d_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 667000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> @@ -1884,6 +1904,7 @@ static struct clk_regmap vapb = {
>  			&vapb_div.hw
>  		},
>  		.num_parents = 1,
> +		.max_rate = 400000000,
>  		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
> diff --git a/drivers/clk/meson/c3-pll.c b/drivers/clk/meson/c3-pll.c
> index 35fda31a19e2..d80d6ee2409d 100644
> --- a/drivers/clk/meson/c3-pll.c
> +++ b/drivers/clk/meson/c3-pll.c
> @@ -286,6 +286,8 @@ static struct clk_regmap gp0_pll_dco = {
>  			.fw_name = "top",
>  		},
>  		.num_parents = 1,
> +		.min_rate = 3000000000,
> +		.max_rate = 6000000000,
>  	},
>  };
>  
> @@ -370,6 +372,8 @@ static struct clk_regmap hifi_pll_dco = {
>  			.fw_name = "top",
>  		},
>  		.num_parents = 1,
> +		.min_rate = 3000000000,
> +		.max_rate = 6000000000,
>  	},
>  };

-- 
Jerome

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ