linux-kernel - Re: [PATCH 2/3] thermal: Add Mediatek thermal controller support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5602EFF0.8040207@mentor.com>
Date:	Wed, 23 Sep 2015 21:31:12 +0300
From:	Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>
To:	Sascha Hauer <s.hauer@...gutronix.de>,
	Eduardo Valentin <edubezval@...il.com>
CC:	<linux-pm@...r.kernel.org>, Zhang Rui <rui.zhang@...el.com>,
	<linux-kernel@...r.kernel.org>, <kernel@...gutronix.de>,
	<linux-mediatek@...ts.infradead.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	Matthias Brugger <matthias.bgg@...il.com>,
	<devicetree@...r.kernel.org>, <mark.rutland@....com>,
	<robh+dt@...nel.org>
Subject: Re: [PATCH 2/3] thermal: Add Mediatek thermal controller support

Hi Sascha,

On 23.09.2015 16:37, Sascha Hauer wrote:
> This adds support for the Mediatek thermal controller found on MT8173
> and likely other SoCs.
> The controller is a bit special. It does not have its own ADC, instead
> it controls the on-SoC AUXADC via AHB bus accesses. For this reason
> we need the physical address of the AUXADC. Also it controls a mux
> using AHB bus accesses, so we need the APMIXEDSYS physical address aswell.
> 
> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> Reviewed-by: Daniel Kurtz <djkurtz@...omium.org>
> ---
>  drivers/thermal/Kconfig       |   8 +
>  drivers/thermal/Makefile      |   1 +
>  drivers/thermal/mtk_thermal.c | 537 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 546 insertions(+)
>  create mode 100644 drivers/thermal/mtk_thermal.c
> 

[snip]

> +
> +	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
> +	if (!auxadc) {
> +		dev_err(&pdev->dev, "missing auxadc node\n");
> +		return -ENODEV;
> +	}
> +
> +	auxadc_phys_base = of_get_phys_base(auxadc);

in case of OF_DYNAMIC enabled of_parse_phandle() requires of_node_put(),
which is fine to place right here.

> +	if (auxadc_phys_base == OF_BAD_ADDR) {
> +		dev_err(&pdev->dev, "Can't get auxadc phys address\n");
> +		return -EINVAL;
> +	}
> +

[snip]

> +
> +	/*
> +	 * These calibration values should finally be provided by the
> +	 * firmware or fuses. For now use default values.
> +	 */
> +	mt->calib_slope = -123;
> +	mt->calib_offset = 465124;
> +
> +	for (i = 0; i < MT8173_NUM_ZONES; i++)
> +		mtk_thermal_init_bank(mt, i, apmixed_phys_base, auxadc_phys_base);
> +
> +	platform_set_drvdata(pdev, mt);
> +
> +	for (i = 0; i < MT8173_NUM_ZONES; i++) {
> +		struct mtk_thermal_bank *bank = &mt->banks[i];
> +
> +		bank->tzd = thermal_zone_of_sensor_register(&pdev->dev, i, bank,
> +				&mtk_thermal_ops);

I would propose to add return value checks here, otherwise there might
be an oops in mtk_thermal_remove(), if something goes wrong.

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/