On 12/19/2013 05:49 AM, Paul Walmsley wrote:
> Add basic platform driver support for the fast CPU cluster DFLL
> clocksource found on Tegra114 SoCs. This small driver selects the
> appropriate Tegra114-specific characterization data and integration
> code. It relies on the DFLL common code to do most of the work.
> diff --git a/drivers/clk/tegra/Kconfig b/drivers/clk/tegra/Kconfig
> +config CLK_TEGRA114_DFLL_FCPU
> + tristate "Clock driver for the Tegra T114 DFLL FCPU"
depends ARCH_TEGRA_114_SOC?
> +CFLAGS_clk-tegra114-dfll-fcpu.o += -Wall -Wextra
> -Wno-unused-parameter \
> + -Wno-missing-field-initializers \
> + -Wno-sign-compare
I'd drop this too.
> diff --git a/drivers/clk/tegra/clk-tegra114-dfll-fcpu.c
> b/drivers/clk/tegra/clk-tegra114-dfll-fcpu.c
> + * clk-tegra114-dfll-fcpu.c - Tegra114 DFLL FCPU clock source driver
I'd drop the filename.
> + * Copyright (C) 2012-2013 NVIDIA Corporation. All rights reserved.
This is 2012-2013 whereas the other files were only 2013?
> +#define DROOP_RATE_MIN 1000000
> +
> +
Double blank line.
> +static int tegra114_dfll_fcpu_probe(struct platform_device *pdev)
> +{
> + struct tegra_dfll_soc_data soc;
Since most of the data in that struct is static, why not just make it a
static global, and initialize the constant fields. That would avoid the
need to copy it inside tegra_dfll_register(), and avoid the need to
assign the fields later in this function.
> + int r, speedo_id, process_id;
> +
> + speedo_id = tegra_get_cpu_speedo_id();
> + process_id = tegra_get_cpu_process_id();
> +
> + memset(&soc, 0, sizeof(soc));
> + soc.driver_name = DRIVER_NAME;
Why is that needed? Shouldn't the library code be using
dev_name(&pdev->dev) or similar?
> + r = tegra_dfll_register(pdev, &soc);
> + if (!r)
> + return r;
> +
> + return 0;
if (!r) means if (r == 0) so this always returns 0. I assume that meant
if (r), but even simpler would be:
return tegra_dfll_register(...);
> +static struct platform_driver tegra114_dfll_fcpu_driver = {
...
> +};
> +
> +module_platform_driver(tegra114_dfll_fcpu_driver);
The blank line before module_platform_driver() is customarily omitted.
> +MODULE_LICENSE("GPL");
"GPL v2"
> +MODULE_ALIAS("platform:" DRIVER_NAME);
That isn't needed since we only probe using DT now.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
