17.06.2019 12:35, Thierry Reding пишет:
> On Mon, Jun 17, 2019 at 02:35:42AM +0300, Dmitry Osipenko wrote:
>> A proper External Memory Controller clock rounding and parent selection
>> functionality is required by the EMC drivers. It is not available using
>> the generic clock implementation, hence add a custom one. The clock rate
>> rounding shall be done by the EMC drivers because they have information
>> about available memory timings, so the drivers will have to register a
>> callback that will round the requested rate. EMC clock users won't be able
>> to request EMC clock by getting -EPROBE_DEFER until EMC driver is probed
>> and the callback is set up. The functionality is somewhat similar to the
>> clk-emc.c which serves Tegra124+ SoC's, the later HW generations support
>> more parent clock sources and the HW configuration and integration with
>> the EMC drivers differs a tad from the older gens, hence it's not really
>> worth to try to squash everything into a single source file.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>> ---
>> drivers/clk/tegra/Makefile | 2 +
>> drivers/clk/tegra/clk-tegra20-emc.c | 305 ++++++++++++++++++++++++++++
>> drivers/clk/tegra/clk-tegra20.c | 55 ++---
>> drivers/clk/tegra/clk-tegra30.c | 38 +++-
>> drivers/clk/tegra/clk.h | 6 +
>> include/linux/clk/tegra.h | 14 ++
>> 6 files changed, 368 insertions(+), 52 deletions(-)
>> create mode 100644 drivers/clk/tegra/clk-tegra20-emc.c
>>
>> diff --git a/drivers/clk/tegra/Makefile b/drivers/clk/tegra/Makefile
>> index 4812e45c2214..df966ca06788 100644
>> --- a/drivers/clk/tegra/Makefile
>> +++ b/drivers/clk/tegra/Makefile
>> @@ -17,7 +17,9 @@ obj-y += clk-tegra-fixed.o
>> obj-y += clk-tegra-super-gen4.o
>> obj-$(CONFIG_TEGRA_CLK_EMC) += clk-emc.o
>> obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20.o
>> +obj-$(CONFIG_ARCH_TEGRA_2x_SOC) += clk-tegra20-emc.o
>> obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra30.o
>> +obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o
>> obj-$(CONFIG_ARCH_TEGRA_114_SOC) += clk-tegra114.o
>> obj-$(CONFIG_ARCH_TEGRA_124_SOC) += clk-tegra124.o
>> obj-$(CONFIG_TEGRA_CLK_DFLL) += clk-tegra124-dfll-fcpu.o
>> diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
>> new file mode 100644
>> index 000000000000..b7f64ad5c04c
>> --- /dev/null
>> +++ b/drivers/clk/tegra/clk-tegra20-emc.c
>> @@ -0,0 +1,305 @@
>> +// SPDX-License-Identifier: GPL-2.0
>
> Perhaps you want to add copyright information here? Part of this is
> copied from other drivers, so keep that copyright intact. But there's
> also quite a bit of new code here, so also make sure to add yourself.
Okay! And it's true that I initially used clk-emc as a template.
[snip]
>> +void tegra30_clk_set_emc_round_callback(tegra30_clk_emc_round_cb *round_cb,
>> + void *cb_arg)
>> +{
>> + tegra20_clk_set_emc_round_callback(round_cb, cb_arg);
>> +}
>> +
>> +bool tegra30_clk_emc_driver_available(struct clk_hw *emc_hw)
>> +{
>> + return tegra20_clk_emc_driver_available(emc_hw);
>> +}
>
> Do we really need to make this distinction? Do you have any work in
> progress patches that would need to override these Tegra30 specific bits
> by code that's not the same as the Tegra20 variant? I don't see why you
> would want to duplicate this if there's no use to it. Or perhaps I'm
> missing something?
There are no other patches planned for this code. The primary reason for the
distinction is that I don't like to have T20 functions mixed with T30 because this
leads to inconsistency and confusion.
[snip]
> Again, I don't see any advantage in quirky things like this. It seems to
> me like the only reason why this exists is so that Tegra30 code doesn't
> have to call functions that start with a tegra20_ prefix. However, we
> already have code that does similar things elsewhere, so I think this
> can be considered "common" practice. No need for this duplication.
Oh, well. But this is not a very good practice in my opinion. I'll adhere to yours
comment in v5.
> Again, if I'm missing something please let me know. Might be worth
> noting why this is done in a code comment or the commit message.
You got everything right.
