Re: [PATCH v6 07/14] clk: tegra: Implement Tegra210 EMC clock

From: Dmitry Osipenko <digetx@gmail.com>
To: Thierry Reding <thierry.reding@gmail.com>,
	Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org, Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Jon Hunter <jonathanh@nvidia.com>, Joseph Lo <josephl@nvidia.com>,
	linux-tegra@vger.kernel.org, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 07/14] clk: tegra: Implement Tegra210 EMC clock
Date: Thu, 9 Apr 2020 21:24:31 +0300	[thread overview]
Message-ID: <8dc000fb-8867-cf8f-8204-a9e1e79a4811@gmail.com> (raw)
In-Reply-To: <20200409175238.3586487-8-thierry.reding@gmail.com>

09.04.2020 20:52, Thierry Reding пишет:
...
> +static long tegra210_clk_emc_round_rate(struct clk_hw *hw, unsigned long rate,
> +					unsigned long *prate)
> +{
> +	struct tegra210_clk_emc *emc = to_tegra210_clk_emc(hw);
> +	struct tegra210_clk_emc_provider *provider = emc->provider;
> +	unsigned int i;
> +
> +	if (!provider || !provider->configs || provider->num_configs == 0)
> +		return clk_hw_get_rate(hw);

This still looks wrong to me. Nobody should be able to get EMC clock
until provider is registered.

This is troublesome, especially given that you're allowing the EMC
driver to be compiled as a loadable module. For example, this won't work
with the current ACTMON driver because it builds OPP table based on the
clk-rate rounding during the driver's probe, so it won't be able to do
it properly if provider is "temporarily" missing.

... I think that in a longer run we should stop manually building the
ACTMON's OPP table and instead define a proper OPP table (per-HW Speedo
ID, with voltages) in a device-tree. But this is just a vague plans for
the future for now.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel