Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423008Ab3FUOb3 (ORCPT ); Fri, 21 Jun 2013 10:31:29 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:38852 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161535Ab3FUOb1 (ORCPT ); Fri, 21 Jun 2013 10:31:27 -0400 Message-ID: <51C463B4.9070506@ti.com> Date: Fri, 21 Jun 2013 10:31:16 -0400 From: Eduardo Valentin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: amit daniel kachhap CC: Eduardo Valentin , , Zhang Rui , , , Kukjin Kim , Subject: Re: [PATCH V6 08/30] thermal: exynos: Add missing definations and code cleanup References: <1371451599-31035-1-git-send-email-amit.daniel@samsung.com> <1371451599-31035-9-git-send-email-amit.daniel@samsung.com> <51C20CCC.3060502@ti.com> In-Reply-To: X-Enigmail-Version: 1.5.1 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2ECWEFKCHGBFAKLBVBUXB" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7767 Lines: 224 ------enig2ECWEFKCHGBFAKLBVBUXB Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 20-06-2013 22:01, amit daniel kachhap wrote: > Hi Eduardo, >=20 > On Thu, Jun 20, 2013 at 1:25 AM, Eduardo Valentin > wrote: >> On 17-06-2013 02:46, Amit Daniel Kachhap wrote: >>> This patch adds some extra register bitfield definations and cleans >>> up the code to prepare for moving register macros and definations ins= ide >>> the TMU data section. >>> >>> Acked-by: Kukjin Kim >>> Acked-by: Jonghwa Lee >>> Signed-off-by: Amit Daniel Kachhap >>> --- >>> drivers/thermal/samsung/exynos_tmu.c | 62 ++++++++++++++++++++++++= +--------- >>> 1 files changed, 46 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/thermal/samsung/exynos_tmu.c b/drivers/thermal/s= amsung/exynos_tmu.c >>> index 5df04a1..fa33a48 100644 >>> --- a/drivers/thermal/samsung/exynos_tmu.c >>> +++ b/drivers/thermal/samsung/exynos_tmu.c >>> @@ -43,9 +43,12 @@ >>> >>> #define EXYNOS_TMU_TRIM_TEMP_MASK 0xff >>> #define EXYNOS_TMU_GAIN_SHIFT 8 >>> +#define EXYNOS_TMU_GAIN_MASK 0xf >>> #define EXYNOS_TMU_REF_VOLTAGE_SHIFT 24 >>> -#define EXYNOS_TMU_CORE_ON 3 >>> -#define EXYNOS_TMU_CORE_OFF 2 >>> +#define EXYNOS_TMU_REF_VOLTAGE_MASK 0x1f >>> +#define EXYNOS_TMU_BUF_SLOPE_SEL_MASK 0xf >>> +#define EXYNOS_TMU_BUF_SLOPE_SEL_SHIFT 8 >>> +#define EXYNOS_TMU_CORE_EN_SHIFT 0 >>> #define EXYNOS_TMU_DEF_CODE_TO_TEMP_OFFSET 50 >>> >>> /* Exynos4210 specific registers */ >>> @@ -63,6 +66,7 @@ >>> #define EXYNOS4210_TMU_TRIG_LEVEL1_MASK 0x10 >>> #define EXYNOS4210_TMU_TRIG_LEVEL2_MASK 0x100 >>> #define EXYNOS4210_TMU_TRIG_LEVEL3_MASK 0x1000 >>> +#define EXYNOS4210_TMU_TRIG_LEVEL_MASK 0x1111 >>> #define EXYNOS4210_TMU_INTCLEAR_VAL 0x1111 >>> >>> /* Exynos5250 and Exynos4412 specific registers */ >>> @@ -72,17 +76,30 @@ >>> #define EXYNOS_EMUL_CON 0x80 >>> >>> #define EXYNOS_TRIMINFO_RELOAD 0x1 >>> +#define EXYNOS_TRIMINFO_SHIFT 0x0 >>> +#define EXYNOS_TMU_RISE_INT_MASK 0x111 >>> +#define EXYNOS_TMU_RISE_INT_SHIFT 0 >>> +#define EXYNOS_TMU_FALL_INT_MASK 0x111 >>> +#define EXYNOS_TMU_FALL_INT_SHIFT 12 >>> #define EXYNOS_TMU_CLEAR_RISE_INT 0x111 >>> #define EXYNOS_TMU_CLEAR_FALL_INT (0x111 << 12) >>> -#define EXYNOS_MUX_ADDR_VALUE 6 >>> -#define EXYNOS_MUX_ADDR_SHIFT 20 >>> #define EXYNOS_TMU_TRIP_MODE_SHIFT 13 >>> +#define EXYNOS_TMU_TRIP_MODE_MASK 0x7 >>> + >>> +#define EXYNOS_TMU_INTEN_RISE0_SHIFT 0 >>> +#define EXYNOS_TMU_INTEN_RISE1_SHIFT 4 >>> +#define EXYNOS_TMU_INTEN_RISE2_SHIFT 8 >>> +#define EXYNOS_TMU_INTEN_RISE3_SHIFT 12 >>> +#define EXYNOS_TMU_INTEN_FALL0_SHIFT 16 >>> +#define EXYNOS_TMU_INTEN_FALL1_SHIFT 20 >>> +#define EXYNOS_TMU_INTEN_FALL2_SHIFT 24 >>> >>> #define EFUSE_MIN_VALUE 40 >>> #define EFUSE_MAX_VALUE 100 >>> >>> #ifdef CONFIG_THERMAL_EMULATION >>> #define EXYNOS_EMUL_TIME 0x57F0 >>> +#define EXYNOS_EMUL_TIME_MASK 0xffff >>> #define EXYNOS_EMUL_TIME_SHIFT 16 >>> #define EXYNOS_EMUL_DATA_SHIFT 8 >>> #define EXYNOS_EMUL_DATA_MASK 0xFF >>> @@ -261,24 +278,37 @@ static void exynos_tmu_control(struct platform_= device *pdev, bool on) >>> mutex_lock(&data->lock); >>> clk_enable(data->clk); >>> >>> - con =3D pdata->reference_voltage << EXYNOS_TMU_REF_VOLTAGE_SHIF= T | >>> - pdata->gain << EXYNOS_TMU_GAIN_SHIFT; >>> + con =3D readl(data->base + EXYNOS_TMU_REG_CONTROL); >>> >>> - if (data->soc =3D=3D SOC_ARCH_EXYNOS) { >>> - con |=3D pdata->noise_cancel_mode << EXYNOS_TMU_TRIP_MO= DE_SHIFT; >>> - con |=3D (EXYNOS_MUX_ADDR_VALUE << EXYNOS_MUX_ADDR_SHIF= T); >>> + if (pdata->reference_voltage) { >>> + con &=3D ~(EXYNOS_TMU_REF_VOLTAGE_MASK << >>> + EXYNOS_TMU_REF_VOLTAGE_SHIFT); >>> + con |=3D pdata->reference_voltage << EXYNOS_TMU_REF_VOL= TAGE_SHIFT; >>> + } >>> + >>> + if (pdata->gain) { >>> + con &=3D ~(EXYNOS_TMU_GAIN_MASK << EXYNOS_TMU_GAIN_SHIF= T); >>> + con |=3D (pdata->gain << EXYNOS_TMU_GAIN_SHIFT); >>> + } >>> + >>> + if (pdata->noise_cancel_mode) { >>> + con &=3D ~(EXYNOS_TMU_TRIP_MODE_MASK << >>> + EXYNOS_TMU_TRIP_MODE_SHIFT); >>> + con |=3D (pdata->noise_cancel_mode << EXYNOS_TMU_TRIP_M= ODE_SHIFT); >>> } >>> >>> if (on) { >>> - con |=3D EXYNOS_TMU_CORE_ON; >> >> >> >> Before, in order to turn core on you had: >> con =3D con | 3; >> >> now you do: >> con =3D con | (1 << 0); >> >> To me, before you would set bit 1 and 0, now you set bit 0. >> >> >>> - interrupt_en =3D pdata->trigger_level3_en << 12 | >>> - pdata->trigger_level2_en << 8 | >>> - pdata->trigger_level1_en << 4 | >>> - pdata->trigger_level0_en; >>> + con |=3D (1 << EXYNOS_TMU_CORE_EN_SHIFT); >>> + interrupt_en =3D >>> + pdata->trigger_level3_en << EXYNOS_TMU_INTEN_RISE3_SHIF= T | >>> + pdata->trigger_level2_en << EXYNOS_TMU_INTEN_RISE2_SHIF= T | >>> + pdata->trigger_level1_en << EXYNOS_TMU_INTEN_RISE1_SHIF= T | >>> + pdata->trigger_level0_en << EXYNOS_TMU_INTEN_RISE0_SHIF= T; >>> if (pdata->threshold_falling) >>> - interrupt_en |=3D interrupt_en << 16; >>> + interrupt_en |=3D >>> + interrupt_en << EXYNOS_TMU_INTEN_FALL0_= SHIFT; >>> } else { >>> - con |=3D EXYNOS_TMU_CORE_OFF; >>> + con &=3D ~(1 << EXYNOS_TMU_CORE_EN_SHIFT); >> >> >> Before, in order to turno core off you had: >> con =3D con | 2; >> >> now you do: >> con =3D con & ~(1 << 0); >> >> To me, before you would set bit 2, now you clear bit 0. >> >> Using the approach on this patch looks correct to me if you have 1 bit= >> core_en for instance. >> >> so, Is this a fix? >> >> Just to be clear, is this what you want ? > Yes you are right. Bit 0 is the actual enable bit. Bit 1 is the > reserve bit and its default value is 1. Earlier both bits are used to > turn on/off the controller which was wrong so this patch rectifies it. Care to mention this in your patch description? It is good for code histo= ry. >=20 > Thanks, > Amit >> >>> interrupt_en =3D 0; /* Disable all interrupts */ >>> } >>> writel(interrupt_en, data->base + EXYNOS_TMU_REG_INTEN); >>> >> >> >> -- >> You have got to be excited about what you are doing. (L. Lamport) >> >> Eduardo Valentin >> >=20 >=20 --=20 You have got to be excited about what you are doing. (L. Lamport) Eduardo Valentin ------enig2ECWEFKCHGBFAKLBVBUXB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iF4EAREIAAYFAlHEY7QACgkQCXcVR3XQvP3KrQD/e3cTK2gGyeVIJwzFlbmACY82 aAPdd9SkIOCsyotCoEwA+QHD6BMhz+2DPpbRQsBsxFV/yq36kUrvPAsMyGH8IRJ4 =EKt4 -----END PGP SIGNATURE----- ------enig2ECWEFKCHGBFAKLBVBUXB-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/