On Mon, Jan 18, 2016 at 06:03:46PM +0800, Wei Ni wrote:
> Add Tegra210 specific SOC_THERM driver.
>
> Signed-off-by: Wei Ni <wni@xxxxxxxxxx>
> ---
> drivers/thermal/tegra/Makefile | 1 +
> drivers/thermal/tegra/soctherm-fuse.c | 11 ++
> drivers/thermal/tegra/soctherm.c | 6 +
> drivers/thermal/tegra/soctherm.h | 4 +
> drivers/thermal/tegra/tegra210-soctherm.c | 181 ++++++++++++++++++++++++++++++
> 5 files changed, 203 insertions(+)
> create mode 100644 drivers/thermal/tegra/tegra210-soctherm.c
This looks pretty good, just a few minor nits...
> diff --git a/drivers/thermal/tegra/soctherm.h b/drivers/thermal/tegra/soctherm.h
[...]
> index 1ac66cafb392..bd0f03bc9d80 100644
> --- a/drivers/thermal/tegra/soctherm.h
> +++ b/drivers/thermal/tegra/soctherm.h
> @@ -108,5 +108,9 @@ int tegra_soctherm_calculate_tsensor_calibration(
> extern struct tegra_soctherm_soc tegra124_soctherm;
> #endif
>
> +#ifdef CONFIG_ARCH_TEGRA_210_SOC
> +extern struct tegra_soctherm_soc tegra210_soctherm;
I would've expected this to be "const".
> diff --git a/drivers/thermal/tegra/tegra210-soctherm.c b/drivers/thermal/tegra/tegra210-soctherm.c
[...]
> +static const struct tegra_tsensor_group tegra210_tsensor_group_cpu = {
> + .id = TEGRA124_SOCTHERM_SENSOR_CPU,
> + .name = "cpu",
> + .sensor_temp_offset = SENSOR_TEMP1,
> + .sensor_temp_mask = SENSOR_TEMP1_CPU_TEMP_MASK,
> + .pdiv = 8,
> + .pdiv_ate = 8,
> + .pdiv_mask = SENSOR_PDIV_CPU_MASK,
> + .pllx_hotspot_diff = 10,
> + .pllx_hotspot_mask = SENSOR_HOTSPOT_CPU_MASK,
> +};
I prefer single spaces for padding because I find it easier to read that
way. Also using tabs to make this look like a table has the drawback
that you run the risk of having to adjust padding for all fields when a
new field is added whose name is longer than any existing ones. Or you
have to rely on excessive padding (such as in this case) to make that
unlikely. So I don't see any advantage in this, but I don't have very
strong objections either.
> +static const struct tegra_tsensor_group *
> +tegra210_tsensor_groups[] = {
Why wrap these two lines? They seem to fit on one line and within 78
columns.
> +static struct tegra_tsensor tegra210_tsensors[] = {
"static const"?
> + {
> + .name = "cpu0",
> + .base = 0xc0,
> + .config = &tegra210_tsensor_config,
> + .calib_fuse_offset = 0x098,
> + .fuse_corr_alpha = 1085000,
> + .fuse_corr_beta = 3244200,
> + .group = &tegra210_tsensor_group_cpu,
> + },
> + {
"}," and "{" can go on the same line.
> +struct tegra_soctherm_soc tegra210_soctherm = {
"const"?
Thierry
Attachment:
signature.asc
Description: PGP signature
