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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html