From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753883AbbCEFpD (ORCPT ); Thu, 5 Mar 2015 00:45:03 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:64099 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753344AbbCEFo4 (ORCPT ); Thu, 5 Mar 2015 00:44:56 -0500 X-AuditID: cbfee690-f79ab6d0000046f7-7c-54f7ed563912 Message-id: <54F7ED56.8020403@samsung.com> Date: Thu, 05 Mar 2015 14:44:54 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-version: 1.0 To: Lukasz Majewski Cc: rui.zhang@intel.com, edubezval@gmail.com, kgene@kernel.org, b.zolnierkie@samsung.com, amit.daniel@samsung.com, a.kesavan@samsung.com, inki.dae@samsung.com, chanho61.park@samsung.com, kyungmin.park@samsung.com, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] thermal: exynos: Add the support for Exynos5433 TMU References: <1424950521-6236-1-git-send-email-cw00.choi@samsung.com> <1424950521-6236-2-git-send-email-cw00.choi@samsung.com> <20150304113831.6da39aa8@amdc2363> In-reply-to: <20150304113831.6da39aa8@amdc2363> Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrNIsWRmVeSWpSXmKPExsWyRsSkSDfs7fcQgy+blCwer1nMZNFwNcRi 44z1rBaX92tbzL9yjdVi0v0JLBb9j18zW5xtesNu8ebhZkaLTY+BEpd3zWGz+Nx7hNFixvl9 TBZPHvaxOfB57Jx1l91j8Z6XTB6bVnWyeWxeUu/Rt2UVo8fnTXIBbFFcNimpOZllqUX6dglc Gc2T37IXbPavuHv/J2MDY4tjFyMnh4SAicSRF0sYIWwxiQv31rN1MXJxCAksZZS49nEjC0xR w/ezLBCJ6YwSFxv2QjmvGSXWX1kMVsUroCUxb9sidhCbRUBV4vjrg6wgNhtQfP+LG2wgtqhA mMTK6Veg6gUlfky+B2aLCOhIXPjYwAwylFngLpPE3WkPmEASwgI+EvcabzNCbFvCKLH2YgvY sZwC+hKt16eA2cxA3ftbp7FB2PISm9e8BZskIdDIITF39zQWiJMEJL5NPgRkcwAlZCU2HWCG +E1S4uCKGywTGMVmITlqFpKxs5CMXcDIvIpRNLUguaA4Kb3IRK84Mbe4NC9dLzk/dxMjMI5P /3s2YQfjvQPWhxgFOBiVeHhnbPweIsSaWFZcmXuI0RToionMUqLJ+cBkkVcSb2hsZmRhamJq bGRuaaYkzvta6mewkEB6YklqdmpqQWpRfFFpTmrxIUYmDk6pBkb+8DX3OpRsFX+yNHw+9va/ rU269Ov0zoZD0U8dIq2eX4iJPCLsu29KtvAbi4qw8hMr/AOXb0w6LdJRM0VsguECi4Kls5ec +Fy9SLUo8Y32BfYp325zfdI6f+CoQT3DxLous0IRY4sDO77tNdwkpcd83ocn/54De0vDlycb Uva8lfr8vGx/Y7sSS3FGoqEWc1FxIgDFDl2k3gIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprKKsWRmVeSWpSXmKPExsVy+t9jQd2wt99DDO5cF7N4vGYxk0XD1RCL jTPWs1pc3q9tMf/KNVaLSfcnsFj0P37NbHG26Q27xZuHmxktNj0GSlzeNYfN4nPvEUaLGef3 MVk8edjH5sDnsXPWXXaPxXteMnlsWtXJ5rF5Sb1H35ZVjB6fN8kFsEU1MNpkpCampBYppOYl 56dk5qXbKnkHxzvHm5oZGOoaWlqYKynkJeam2iq5+AToumXmAB2rpFCWmFMKFApILC5W0rfD NCE0xE3XAqYxQtc3JAiux8gADSSsYcxonvyWvWCzf8Xd+z8ZGxhbHLsYOTkkBEwkGr6fZYGw xSQu3FvP1sXIxSEkMJ1R4mLDXhYI5zWjxPori8GqeAW0JOZtW8QOYrMIqEocf32QFcRmA4rv f3GDDcQWFQiTWDn9ClS9oMSPyffAbBEBHYkLHxuYQYYyC9xlkrg77QETSEJYwEfiXuNtRoht Sxgl1l5sYQRJcAroS7RenwJmMwN172+dxgZhy0tsXvOWeQKjwCwkS2YhKZuFpGwBI/MqRtHU guSC4qT0XCO94sTc4tK8dL3k/NxNjOAk8Ux6B+OqBotDjAIcjEo8vB83fw8RYk0sK67MPcQo wcGsJMI76TRQiDclsbIqtSg/vqg0J7X4EKMpMAwmMkuJJucDE1heSbyhsYmZkaWRuaGFkbG5 kjivkn1biJBAemJJanZqakFqEUwfEwenVANjaZKO/eME72+1xtukdSx4z51LUowx1oznebLn 2m+7qwvlRC33/OHu8meMMFDNrC/tLQ8ufaM/0/qp6QbNz+sDTyt9vcrxyFBI79iRM/yyP+Zf WWRyv9pLesralM38HEWbb17mmiP8UzYgLnPRzW0s5wJ4HeJ7J8h0uKiekuIzfHfzdr+oQogS S3FGoqEWc1FxIgBMaGi2KAMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Lukasz, On 03/04/2015 07:38 PM, Lukasz Majewski wrote: > Hi Chanwoo, > >> This patch adds the support for Exynos5433's TMU (Thermal Management >> Unit). Exynos5433 has a little different register bit fields as >> following description: >> - Support the eight trip points for rising/falling interrupt by using >> two registers >> - Read the calibration type (1-point or 2-point) and sensor id from >> TRIMINFO register >> - Use a little different register address >> >> Cc: Zhang Rui >> Cc: Eduardo Valentin >> Signed-off-by: Chanwoo Choi >> --- >> drivers/thermal/samsung/exynos_tmu.c | 161 >> +++++++++++++++++++++++++++++++++-- >> drivers/thermal/samsung/exynos_tmu.h | 1 + 2 files changed, 157 >> insertions(+), 5 deletions(-) >> >> diff --git a/drivers/thermal/samsung/exynos_tmu.c >> b/drivers/thermal/samsung/exynos_tmu.c index 1d30b09..1bb2fb7 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.c >> +++ b/drivers/thermal/samsung/exynos_tmu.c >> @@ -97,6 +97,32 @@ >> #define EXYNOS4412_MUX_ADDR_VALUE 6 >> #define EXYNOS4412_MUX_ADDR_SHIFT 20 >> >> +/* Exynos5433 specific registers */ >> +#define EXYNOS5433_TMU_REG_CONTROL1 0x024 >> +#define EXYNOS5433_TMU_SAMPLING_INTERVAL 0x02c >> +#define EXYNOS5433_TMU_COUNTER_VALUE0 0x030 >> +#define EXYNOS5433_TMU_COUNTER_VALUE1 0x034 >> +#define EXYNOS5433_TMU_REG_CURRENT_TEMP1 0x044 >> +#define EXYNOS5433_THD_TEMP_RISE3_0 0x050 >> +#define EXYNOS5433_THD_TEMP_RISE7_4 0x054 >> +#define EXYNOS5433_THD_TEMP_FALL3_0 0x060 >> +#define EXYNOS5433_THD_TEMP_FALL7_4 0x064 >> +#define EXYNOS5433_TMU_REG_INTEN 0x0c0 >> +#define EXYNOS5433_TMU_REG_INTPEND 0x0c8 >> +#define EXYNOS5433_TMU_EMUL_CON 0x110 >> +#define EXYNOS5433_TMU_PD_DET_EN 0x130 >> + >> +#define EXYNOS5433_TRIMINFO_SENSOR_ID_SHIFT 16 >> +#define EXYNOS5433_TRIMINFO_CALIB_SEL_SHIFT 23 >> +#define EXYNOS5433_TRIMINFO_SENSOR_ID_MASK \ >> + (0xf << EXYNOS5433_TRIMINFO_SENSOR_ID_SHIFT) >> +#define EXYNOS5433_TRIMINFO_CALIB_SEL_MASK BIT(23) >> + >> +#define EXYNOS5433_TRIMINFO_ONE_POINT_TRIMMING 0 >> +#define EXYNOS5433_TRIMINFO_TWO_POINT_TRIMMING 1 >> + >> +#define EXYNOS5433_PD_DET_EN 1 >> + >> /*exynos5440 specific registers*/ >> #define EXYNOS5440_TMU_S0_7_TRIM 0x000 >> #define EXYNOS5440_TMU_S0_7_CTRL 0x020 >> @@ -484,6 +510,101 @@ out: >> return ret; >> } >> >> +static int exynos5433_tmu_initialize(struct platform_device *pdev) >> +{ >> + struct exynos_tmu_data *data = platform_get_drvdata(pdev); >> + struct exynos_tmu_platform_data *pdata = data->pdata; >> + struct thermal_zone_device *tz = data->tzd; >> + unsigned int status, trim_info; >> + unsigned int rising_threshold = 0, falling_threshold = 0; >> + unsigned long temp, temp_hist; >> + int ret = 0, threshold_code, i, sensor_id, cal_type; >> + >> + status = readb(data->base + EXYNOS_TMU_REG_STATUS); >> + if (!status) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO); >> + sanitize_temp_error(data, trim_info); >> + >> + /* Read the temperature sensor id */ >> + sensor_id = (trim_info & EXYNOS5433_TRIMINFO_SENSOR_ID_MASK) >> + >> >> EXYNOS5433_TRIMINFO_SENSOR_ID_SHIFT; >> + dev_info(&pdev->dev, "Temperature sensor ID: 0x%x\n", >> sensor_id); + > > Isn't dev_info a bit too noisy? IMHO, dev_dbg would be enough > here. > > Please consider this globally. OK, I'll use dev_dbg. > >> + /* Read the calibration mode */ >> + writel(trim_info, data->base + EXYNOS_TMU_REG_TRIMINFO); >> + cal_type = (trim_info & EXYNOS5433_TRIMINFO_CALIB_SEL_MASK) >> + >> >> EXYNOS5433_TRIMINFO_CALIB_SEL_SHIFT; + >> + switch (cal_type) { >> + case EXYNOS5433_TRIMINFO_ONE_POINT_TRIMMING: >> + pdata->cal_type = TYPE_ONE_POINT_TRIMMING; >> + break; >> + case EXYNOS5433_TRIMINFO_TWO_POINT_TRIMMING: >> + pdata->cal_type = TYPE_TWO_POINT_TRIMMING; >> + break; >> + default: >> + pdata->cal_type = TYPE_ONE_POINT_TRIMMING; >> + break; >> + }; >> + >> + dev_info(&pdev->dev, "Calibration type is %d-point >> calibration\n", >> + cal_type ? 2 : 1); >> + >> + /* Write temperature code for rising and falling threshold */ >> + for (i = 0; i < of_thermal_get_ntrips(tz); i++) { >> + int rising_reg_offset, falling_reg_offset; >> + int j = 0; >> + >> + switch (i) { >> + case 0: >> + case 1: >> + case 2: >> + case 3: >> + rising_reg_offset = >> EXYNOS5433_THD_TEMP_RISE3_0; >> + falling_reg_offset = >> EXYNOS5433_THD_TEMP_FALL3_0; >> + j = i; >> + break; >> + case 4: >> + case 5: >> + case 6: >> + case 7: >> + rising_reg_offset = >> EXYNOS5433_THD_TEMP_RISE7_4; >> + falling_reg_offset = >> EXYNOS5433_THD_TEMP_FALL7_4; >> + j = i - 4; >> + break; >> + default: >> + continue; >> + } >> + >> + /* Write temperature code for rising threshold */ >> + tz->ops->get_trip_temp(tz, i, &temp); >> + temp /= MCELSIUS; >> + threshold_code = temp_to_code(data, temp); >> + >> + rising_threshold = readl(data->base + >> rising_reg_offset); >> + rising_threshold |= (threshold_code << j * 8); >> + writel(rising_threshold, data->base + >> rising_reg_offset); + >> + /* Write temperature code for falling threshold */ >> + tz->ops->get_trip_hyst(tz, i, &temp_hist); >> + temp_hist = temp - (temp_hist / MCELSIUS); >> + threshold_code = temp_to_code(data, temp_hist); >> + >> + falling_threshold = readl(data->base + >> falling_reg_offset); >> + falling_threshold &= ~(0xff << j * 8); >> + falling_threshold |= (threshold_code << j * 8); >> + writel(falling_threshold, data->base + >> falling_reg_offset); >> + } >> + >> + data->tmu_clear_irqs(data); >> +out: >> + return ret; >> +} >> + >> static int exynos5440_tmu_initialize(struct platform_device *pdev) >> { >> struct exynos_tmu_data *data = platform_get_drvdata(pdev); >> @@ -682,7 +803,8 @@ static void exynos7_tmu_control(struct >> platform_device *pdev, bool on) >> if (on) { >> con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT); >> - con |= (1 << EXYNOS7_PD_DET_EN_SHIFT); >> + if (data->soc == SOC_ARCH_EXYNOS7) >> + con |= (1 << EXYNOS7_PD_DET_EN_SHIFT); > > Isn't exynos7 implying that we already have SOC_ARCH_EXYNOS7? > Why do we need this extra check? On this patch, Exynos5433 TMU use the exynos7_tmu_control function. But, as below your comment, I'll add the separate function for Exynos5433. > >> interrupt_en = >> (of_thermal_is_trip_valid(tz, 7) >> << EXYNOS7_TMU_INTEN_RISE7_SHIFT) | >> @@ -705,11 +827,20 @@ static void exynos7_tmu_control(struct >> platform_device *pdev, bool on) interrupt_en << >> EXYNOS_TMU_INTEN_FALL0_SHIFT; } else { >> con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT); >> - con &= ~(1 << EXYNOS7_PD_DET_EN_SHIFT); >> + if (data->soc == SOC_ARCH_EXYNOS7) >> + con &= ~(1 << EXYNOS7_PD_DET_EN_SHIFT); >> interrupt_en = 0; /* Disable all interrupts */ >> } >> >> - writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN); >> + if (data->soc == SOC_ARCH_EXYNOS5433) { >> + int pd_det_en = on ? EXYNOS5433_PD_DET_EN : 0; >> + >> + writel(pd_det_en, data->base + >> EXYNOS5433_TMU_PD_DET_EN); >> + writel(interrupt_en, data->base + >> EXYNOS5433_TMU_REG_INTEN); >> + } else { >> + writel(interrupt_en, data->base + >> EXYNOS7_TMU_REG_INTEN); >> + } >> + >> writel(con, data->base + EXYNOS_TMU_REG_CONTROL); >> } >> >> @@ -770,6 +901,8 @@ static void exynos4412_tmu_set_emulation(struct >> exynos_tmu_data *data, >> if (data->soc == SOC_ARCH_EXYNOS5260) >> emul_con = EXYNOS5260_EMUL_CON; >> + if (data->soc == SOC_ARCH_EXYNOS5433) >> + emul_con = EXYNOS5433_TMU_EMUL_CON; >> else if (data->soc == SOC_ARCH_EXYNOS7) >> emul_con = EXYNOS7_TMU_REG_EMUL_CON; >> else >> @@ -882,6 +1015,9 @@ static void exynos4210_tmu_clear_irqs(struct >> exynos_tmu_data *data) } else if (data->soc == SOC_ARCH_EXYNOS7) { >> tmu_intstat = EXYNOS7_TMU_REG_INTPEND; >> tmu_intclear = EXYNOS7_TMU_REG_INTPEND; >> + } else if (data->soc == SOC_ARCH_EXYNOS5433) { >> + tmu_intstat = EXYNOS5433_TMU_REG_INTPEND; >> + tmu_intclear = EXYNOS5433_TMU_REG_INTPEND; >> } else { >> tmu_intstat = EXYNOS_TMU_REG_INTSTAT; >> tmu_intclear = EXYNOS_TMU_REG_INTCLEAR; >> @@ -926,6 +1062,7 @@ static const struct of_device_id >> exynos_tmu_match[] = { { .compatible = "samsung,exynos5260-tmu", }, >> { .compatible = "samsung,exynos5420-tmu", }, >> { .compatible = "samsung,exynos5420-tmu-ext-triminfo", }, >> + { .compatible = "samsung,exynos5433-tmu", }, >> { .compatible = "samsung,exynos5440-tmu", }, >> { .compatible = "samsung,exynos7-tmu", }, >> { /* sentinel */ }, >> @@ -949,6 +1086,8 @@ static int exynos_of_get_soc_type(struct >> device_node *np) else if (of_device_is_compatible(np, >> "samsung,exynos5420-tmu-ext-triminfo")) >> return SOC_ARCH_EXYNOS5420_TRIMINFO; >> + else if (of_device_is_compatible(np, >> "samsung,exynos5433-tmu")) >> + return SOC_ARCH_EXYNOS5433; >> else if (of_device_is_compatible(np, >> "samsung,exynos5440-tmu")) return SOC_ARCH_EXYNOS5440; >> else if (of_device_is_compatible(np, "samsung,exynos7-tmu")) >> @@ -1069,6 +1208,13 @@ static int exynos_map_dt_data(struct >> platform_device *pdev) data->tmu_set_emulation = >> exynos4412_tmu_set_emulation; data->tmu_clear_irqs = >> exynos4210_tmu_clear_irqs; break; >> + case SOC_ARCH_EXYNOS5433: >> + data->tmu_initialize = exynos5433_tmu_initialize; >> + data->tmu_control = exynos7_tmu_control; > > I must frankly admit that I'm a bit confused. > > I'm curious why we didn't either define totally separate set of > exynos5433_tmu_* functions or reusing existing exynos7_tmu_* ? > > Are exynos7 and exynos5433 so much different? Exynos5444 TMU has a bit different register map from Exynos7 TMU. To remove some confusion between Exynos7 and Exnynos5433, I'll add seprate function for Exynos5433 TMU. [snip] Thanks, Chanwoo Choi