Hi Lukasz, On Mon, Jan 19, 2015 at 6:34 PM, Lukasz Majewski <l.majewski@xxxxxxxxxxx> wrote: > Hi Abhilash, > >> Add registers, bit fields and compatible strings for Exynos7 TMU >> (Thermal Management Unit). Following are a few of the differences >> in the Exynos7 TMU from earlier SoCs: >> - 8 trigger levels >> - Different bit offsets and more registers for the rising >> and falling thresholds. >> - New power down detection bit in the TMU_CONTROL register >> which does not update the CURRENT_TEMP0 when tmu power down >> is detected. >> - Change in bit offset for the NEXT_DATA field of EMUL_CON >> register. EMUL_CON register address has also changed. >> - INTSTAT and INTCLEAR registers present in earlier SoCs >> have been combined into one INTPEND register. The register >> address for INTCLEAR and INTPEND is also different. >> - Since there are 8 rising/falling interrupts as against >> at most 4 in earlier SoCs the INTEN bit offsets are different. >> - Multiple probe support which is handled by a TMU_CONTROL1 >> register (No support for this in the current patch). >> >> This patch adds special clock support required only for Exynos7 >> and updates the bindings documentation appropriately. It also updates >> the "code_to_temp" prototype as Exynos7 has 9 bit code-temp mapping. >> >> Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx> >> --- >> This patch is based on Lukasz Majewski's Exynos TMU v4 patchset: >> http://www.spinics.net/lists/linux-samsung-soc/msg41183.html >> >> Changes since v2: >> - Rebased on top of Lukasz' latest exynos tmu series (v4). >> - Added new exynos7 soc_type. >> Changes since v1: >> - Rebased on top of Lukasz' latest exynos tmu series (v2). >> - Added sclk support to patch adding Exynos7 tmu support. >> Previously, it was a separate patch. >> - Used the SOC type to decide if sclk is present. >> >> .../devicetree/bindings/thermal/exynos-thermal.txt | 4 + >> drivers/thermal/samsung/exynos_tmu.c | 203 >> +++++++++++++++++++- >> drivers/thermal/samsung/exynos_tmu.h | 1 + 3 files >> changed, 199 insertions(+), 9 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt >> b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt index >> 0f44932..695150a 100644 --- >> a/Documentation/devicetree/bindings/thermal/exynos-thermal.txt +++ >> b/Documentation/devicetree/bindings/thermal/exynos-thermal.txt @@ >> -12,6 +12,7 @@ "samsung,exynos5420-tmu-ext-triminfo" for TMU channels >> 2, 3 and 4 Exynos5420 (Must pass triminfo base and triminfo clock) >> "samsung,exynos5440-tmu" >> + "samsung,exynos7-tmu" >> - interrupt-parent : The phandle for the interrupt controller >> - reg : Address range of the thermal registers. For soc's which has >> multiple instances of TMU and some registers are shared across all >> TMU's like @@ -32,10 +33,13 @@ >> - clocks : The main clocks for TMU device >> -- 1. operational clock for TMU channel >> -- 2. optional clock to access the shared registers of TMU >> channel >> + -- 3. optional special clock for functional operation >> - clock-names : Thermal system clock name >> -- "tmu_apbif" operational clock for current TMU channel >> -- "tmu_triminfo_apbif" clock to access the shared triminfo >> register for current TMU channel >> + -- "tmu_sclk" clock for functional operation of the current >> TMU >> + channel >> - vtmu-supply: This entry is optional and provides the regulator >> node supplying voltage to TMU. If needed this entry can be placed >> inside board/platform specific dts file. > > I would recommend splitting the documentation from the code. IMHO it > would be more readable. I will split the documentation out into a separate patch. > >> diff --git a/drivers/thermal/samsung/exynos_tmu.c >> b/drivers/thermal/samsung/exynos_tmu.c index 633a9e2..8b26acb 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.c >> +++ b/drivers/thermal/samsung/exynos_tmu.c >> @@ -119,6 +119,26 @@ >> #define EXYNOS5440_TMU_TH_RISE4_SHIFT 24 >> #define EXYNOS5440_EFUSE_SWAP_OFFSET 8 >> >> +/* Exynos7 specific registers */ >> +#define EXYNOS7_THD_TEMP_RISE7_6 0x50 >> +#define EXYNOS7_THD_TEMP_FALL7_6 0x60 >> +#define EXYNOS7_TMU_REG_INTEN 0x110 >> +#define EXYNOS7_TMU_REG_INTPEND 0x118 > ^^^^^ I suppose that > this misalignment is > only done by my mail > client and checkpatch > is not complaining. I re-ran checkpatch and it did not report any warnings, so perhaps it is your e-mail client. >> +#define EXYNOS7_TMU_REG_EMUL_CON 0x160 >> + >> +#define EXYNOS7_TMU_TEMP_MASK 0x1ff >> +#define EXYNOS7_PD_DET_EN_SHIFT 23 >> +#define EXYNOS7_TMU_INTEN_RISE0_SHIFT 0 >> +#define EXYNOS7_TMU_INTEN_RISE1_SHIFT 1 >> +#define EXYNOS7_TMU_INTEN_RISE2_SHIFT 2 >> +#define EXYNOS7_TMU_INTEN_RISE3_SHIFT 3 >> +#define EXYNOS7_TMU_INTEN_RISE4_SHIFT 4 >> +#define EXYNOS7_TMU_INTEN_RISE5_SHIFT 5 >> +#define EXYNOS7_TMU_INTEN_RISE6_SHIFT 6 >> +#define EXYNOS7_TMU_INTEN_RISE7_SHIFT 7 >> +#define EXYNOS7_EMUL_DATA_SHIFT 7 >> +#define EXYNOS7_EMUL_DATA_MASK 0x1ff >> + >> #define MCELSIUS 1000 >> /** >> * struct exynos_tmu_data : A structure to hold the private data of >> the TMU @@ -133,6 +153,7 @@ >> * @lock: lock to implement synchronization. >> * @clk: pointer to the clock structure. >> * @clk_sec: pointer to the clock structure for accessing the >> base_second. >> + * @sclk: pointer to the clock structure for accessing the tmu >> special clk. >> * @temp_error1: fused value of the first point trim. >> * @temp_error2: fused value of the second point trim. >> * @regulator: pointer to the TMU regulator structure. >> @@ -152,8 +173,8 @@ struct exynos_tmu_data { >> enum soc_type soc; >> struct work_struct irq_work; >> struct mutex lock; >> - struct clk *clk, *clk_sec; >> - u8 temp_error1, temp_error2; >> + struct clk *clk, *clk_sec, *sclk; >> + u16 temp_error1, temp_error2; >> struct regulator *regulator; >> struct thermal_zone_device *tzd; >> >> @@ -223,7 +244,7 @@ static int temp_to_code(struct exynos_tmu_data >> *data, u8 temp) >> * Calculate a temperature value from a temperature code. >> * The unit of the temperature is degree Celsius. >> */ >> -static int code_to_temp(struct exynos_tmu_data *data, u8 temp_code) >> +static int code_to_temp(struct exynos_tmu_data *data, u16 temp_code) >> { >> struct exynos_tmu_platform_data *pdata = data->pdata; >> int temp; >> @@ -513,6 +534,81 @@ static int exynos5440_tmu_initialize(struct >> platform_device *pdev) return ret; >> } >> >> +static int exynos7_tmu_initialize(struct platform_device *pdev) >> +{ >> + struct exynos_tmu_data *data = platform_get_drvdata(pdev); >> + struct thermal_zone_device *tz = data->tzd; >> + struct exynos_tmu_platform_data *pdata = data->pdata; >> + unsigned int status, trim_info; >> + unsigned int rising_threshold = 0, falling_threshold = 0; >> + int ret = 0, threshold_code, i; >> + unsigned long temp, temp_hist; >> + unsigned int reg_off, bit_off; >> + >> + status = readb(data->base + EXYNOS_TMU_REG_STATUS); >> + if (!status) { >> + ret = -EBUSY; >> + goto out; >> + } >> + >> + trim_info = readl(data->base + EXYNOS_TMU_REG_TRIMINFO); >> + >> + data->temp_error1 = trim_info & EXYNOS7_TMU_TEMP_MASK; >> + if (!data->temp_error1 || >> + (pdata->min_efuse_value > data->temp_error1) || >> + (data->temp_error1 > pdata->max_efuse_value)) >> + data->temp_error1 = pdata->efuse_value & >> EXYNOS_TMU_TEMP_MASK; + >> + /* Write temperature code for rising and falling threshold */ >> + for (i = (of_thermal_get_ntrips(tz) - 1); i >= 0; i--) { >> + /* >> + * On exynos7 there are 4 rising and 4 falling >> threshold >> + * registers (0x50-0x5c and 0x60-0x6c respectively). >> Each >> + * register holds the value of two threshold levels >> (at bit >> + * offsets 0 and 16). Based on the fact that there >> are atmost >> + * eight possible trigger levels, calculate the >> register and >> + * bit offsets where the threshold levels are to be >> written. >> + * >> + * e.g. EXYNOS7_THD_TEMP_RISE7_6 (0x50) >> + * [24:16] - Threshold level 7 >> + * [8:0] - Threshold level 6 >> + * e.g. EXYNOS7_THD_TEMP_RISE5_4 (0x54) >> + * [24:16] - Threshold level 5 >> + * [8:0] - Threshold level 4 >> + * >> + * and similarly for falling thresholds. >> + */ >> + reg_off = ((7 - i) / 2) * 4; >> + bit_off = ((8 - i) % 2); > I guess that above code is for setting proper > offsets for storing thresholds. A comment > would clarify the potential doubts. Sure, I will make the comment above more descriptive. > >> + >> + tz->ops->get_trip_temp(tz, i, &temp); >> + temp /= MCELSIUS; >> + >> + tz->ops->get_trip_hyst(tz, i, &temp_hist); >> + temp_hist = temp - (temp_hist / MCELSIUS); >> + >> + threshold_code = temp_to_code(data, temp); >> + rising_threshold = readl(data->base + >> + EXYNOS7_THD_TEMP_RISE7_6 + reg_off); >> + rising_threshold &= ~(0x1ff << (16 * bit_off)); > It would be probably enough to > explain what does the 0x1ff mean > in a comment. OK, will do. >> + rising_threshold |= threshold_code << (16 * bit_off); >> + writel(rising_threshold, >> + data->base + EXYNOS7_THD_TEMP_RISE7_6 + >> reg_off); + >> + threshold_code = temp_to_code(data, temp_hist); >> + falling_threshold = readl(data->base + >> + EXYNOS7_THD_TEMP_FALL7_6 + reg_off); >> + falling_threshold &= ~(0x1ff << (16 * bit_off)); > ^^^^^ > The same as above. OK. > >> + falling_threshold |= threshold_code << (16 * >> bit_off); >> + writel(falling_threshold, >> + data->base + EXYNOS7_THD_TEMP_FALL7_6 + >> reg_off); >> + } >> + >> + data->tmu_clear_irqs(data); >> +out: >> + return ret; >> +} >> + >> static void exynos4210_tmu_control(struct platform_device *pdev, >> bool on) { >> struct exynos_tmu_data *data = platform_get_drvdata(pdev); >> @@ -573,6 +669,46 @@ static void exynos5440_tmu_control(struct >> platform_device *pdev, bool on) writel(con, data->base + >> EXYNOS5440_TMU_S0_7_CTRL); } >> >> +static void exynos7_tmu_control(struct platform_device *pdev, bool >> on) +{ >> + struct exynos_tmu_data *data = platform_get_drvdata(pdev); >> + struct thermal_zone_device *tz = data->tzd; >> + unsigned int con, interrupt_en; >> + >> + con = get_con_reg(data, readl(data->base + >> EXYNOS_TMU_REG_CONTROL)); + >> + if (on) { >> + con |= (1 << EXYNOS_TMU_CORE_EN_SHIFT); >> + interrupt_en = >> + (of_thermal_is_trip_valid(tz, 7) >> + << EXYNOS7_TMU_INTEN_RISE7_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 6) >> + << EXYNOS7_TMU_INTEN_RISE6_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 5) >> + << EXYNOS7_TMU_INTEN_RISE5_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 4) >> + << EXYNOS7_TMU_INTEN_RISE4_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 3) >> + << EXYNOS7_TMU_INTEN_RISE3_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 2) >> + << EXYNOS7_TMU_INTEN_RISE2_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 1) >> + << EXYNOS7_TMU_INTEN_RISE1_SHIFT) | >> + (of_thermal_is_trip_valid(tz, 0) >> + << EXYNOS7_TMU_INTEN_RISE0_SHIFT); >> + >> + interrupt_en |= >> + interrupt_en << EXYNOS_TMU_INTEN_FALL0_SHIFT; >> + } else { >> + con &= ~(1 << EXYNOS_TMU_CORE_EN_SHIFT); >> + interrupt_en = 0; /* Disable all interrupts */ >> + } >> + con |= 1 << EXYNOS7_PD_DET_EN_SHIFT; >> + >> + writel(interrupt_en, data->base + EXYNOS7_TMU_REG_INTEN); >> + writel(con, data->base + EXYNOS_TMU_REG_CONTROL); >> +} >> + >> int exynos_get_temp(void *p, long *temp) >> { >> struct exynos_tmu_data *data = p; >> @@ -602,9 +738,19 @@ static u32 get_emul_con_reg(struct >> exynos_tmu_data *data, unsigned int val, val &= >> ~(EXYNOS_EMUL_TIME_MASK << EXYNOS_EMUL_TIME_SHIFT); val |= >> (EXYNOS_EMUL_TIME << EXYNOS_EMUL_TIME_SHIFT); } >> - val &= ~(EXYNOS_EMUL_DATA_MASK << >> EXYNOS_EMUL_DATA_SHIFT); >> - val |= (temp_to_code(data, temp) << >> EXYNOS_EMUL_DATA_SHIFT) | >> - EXYNOS_EMUL_ENABLE; >> + if (data->soc == SOC_ARCH_EXYNOS7) { >> + val &= ~(EXYNOS7_EMUL_DATA_MASK << >> + EXYNOS7_EMUL_DATA_SHIFT); >> + val |= (temp_to_code(data, temp) << >> + EXYNOS7_EMUL_DATA_SHIFT) | >> + EXYNOS_EMUL_ENABLE; >> + } else { >> + val &= ~(EXYNOS_EMUL_DATA_MASK << >> + EXYNOS_EMUL_DATA_SHIFT); >> + val |= (temp_to_code(data, temp) << >> + EXYNOS_EMUL_DATA_SHIFT) | >> + EXYNOS_EMUL_ENABLE; >> + } >> } else { >> val &= ~EXYNOS_EMUL_ENABLE; >> } >> @@ -620,6 +766,8 @@ static void exynos4412_tmu_set_emulation(struct >> exynos_tmu_data *data, >> if (data->soc == SOC_ARCH_EXYNOS5260) >> emul_con = EXYNOS5260_EMUL_CON; >> + else if (data->soc == SOC_ARCH_EXYNOS7) >> + emul_con = EXYNOS7_TMU_REG_EMUL_CON; >> else >> emul_con = EXYNOS_EMUL_CON; >> >> @@ -683,6 +831,12 @@ static int exynos5440_tmu_read(struct >> exynos_tmu_data *data) return readb(data->base + >> EXYNOS5440_TMU_S0_7_TEMP); } >> >> +static int exynos7_tmu_read(struct exynos_tmu_data *data) >> +{ >> + return readw(data->base + EXYNOS_TMU_REG_CURRENT_TEMP) & >> + EXYNOS7_TMU_TEMP_MASK; >> +} >> + >> static void exynos_tmu_work(struct work_struct *work) >> { >> struct exynos_tmu_data *data = container_of(work, >> @@ -721,6 +875,8 @@ static void exynos4210_tmu_clear_irqs(struct >> exynos_tmu_data *data) if (data->soc == SOC_ARCH_EXYNOS5260) { >> tmu_intstat = EXYNOS5260_TMU_REG_INTSTAT; >> tmu_intclear = EXYNOS5260_TMU_REG_INTCLEAR; >> + } else if (data->soc == SOC_ARCH_EXYNOS7) { >> + tmu_intstat = tmu_intclear = EXYNOS7_TMU_REG_INTPEND; >> } else { >> tmu_intstat = EXYNOS_TMU_REG_INTSTAT; >> tmu_intclear = EXYNOS_TMU_REG_INTCLEAR; >> @@ -782,6 +938,9 @@ static const struct of_device_id >> exynos_tmu_match[] = { { >> .compatible = "samsung,exynos5440-tmu", >> }, >> + { >> + .compatible = "samsung,exynos7-tmu", > > I'm a bit concerned with this > "exynos7-tmu" name. > From the exynos4 history (and since we > extract tmu type from the compatible) I > can predict that it would be better to > give the full name of the SoC to be > supported - e.g. exynos7XXX-tmu. Exynos7 is the name of the 64-bit arm SoC for which I am adding TMU support, this has been previously discussed here: http://www.spinics.net/lists/linux-samsung-soc/msg36765.html and in a few other threads. >> + }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, exynos_tmu_match); >> @@ -805,6 +964,8 @@ static int exynos_of_get_soc_type(struct >> device_node *np) return SOC_ARCH_EXYNOS5420_TRIMINFO; >> else if (of_device_is_compatible(np, >> "samsung,exynos5440-tmu")) return SOC_ARCH_EXYNOS5440; >> + else if (of_device_is_compatible(np, "samsung,exynos7-tmu")) >> + return SOC_ARCH_EXYNOS7; >> >> return -EINVAL; >> } >> @@ -928,6 +1089,13 @@ static int exynos_map_dt_data(struct >> platform_device *pdev) data->tmu_set_emulation = >> exynos5440_tmu_set_emulation; data->tmu_clear_irqs = >> exynos5440_tmu_clear_irqs; break; >> + case SOC_ARCH_EXYNOS7: >> + data->tmu_initialize = exynos7_tmu_initialize; >> + data->tmu_control = exynos7_tmu_control; >> + data->tmu_read = exynos7_tmu_read; >> + data->tmu_set_emulation = >> exynos4412_tmu_set_emulation; >> + data->tmu_clear_irqs = exynos4210_tmu_clear_irqs; >> + break; >> default: >> dev_err(&pdev->dev, "Platform not supported\n"); >> return -EINVAL; >> @@ -1017,27 +1185,43 @@ static int exynos_tmu_probe(struct >> platform_device *pdev) goto err_clk_sec; >> } >> >> + if (data->soc == SOC_ARCH_EXYNOS7) { >> + data->sclk = devm_clk_get(&pdev->dev, "tmu_sclk"); >> + if (IS_ERR(data->sclk)) { >> + dev_err(&pdev->dev, "Failed to get sclk\n"); >> + goto err_clk; >> + } else { >> + ret = clk_prepare_enable(data->sclk); >> + if (ret) { >> + dev_err(&pdev->dev, "Failed to >> enable sclk\n"); >> + goto err_clk; >> + } >> + } >> + } >> + >> ret = exynos_tmu_initialize(pdev); >> if (ret) { >> dev_err(&pdev->dev, "Failed to initialize TMU\n"); >> - goto err_clk; >> + goto err_sclk; >> } >> >> ret = devm_request_irq(&pdev->dev, data->irq, exynos_tmu_irq, >> IRQF_TRIGGER_RISING | IRQF_SHARED, >> dev_name(&pdev->dev), data); if (ret) { >> dev_err(&pdev->dev, "Failed to request irq: %d\n", >> data->irq); >> - goto err_clk; >> + goto err_sclk; >> } >> >> ret = exynos_tmu_initialize(pdev); >> if (ret) { >> dev_err(&pdev->dev, "Failed to initialize TMU\n"); >> - goto err_clk; >> + goto err_sclk; >> } >> exynos_tmu_control(pdev, true); >> return 0; >> >> +err_sclk: >> + clk_disable_unprepare(data->sclk); >> err_clk: >> clk_unprepare(data->clk); >> err_clk_sec: >> @@ -1057,6 +1241,7 @@ static int exynos_tmu_remove(struct >> platform_device *pdev) thermal_zone_of_sensor_unregister(&pdev->dev, >> tzd); exynos_tmu_control(pdev, false); >> >> + clk_disable_unprepare(data->sclk); >> clk_unprepare(data->clk); >> if (!IS_ERR(data->clk_sec)) >> clk_unprepare(data->clk_sec); >> diff --git a/drivers/thermal/samsung/exynos_tmu.h >> b/drivers/thermal/samsung/exynos_tmu.h index d876d4c..d7c34bd 100644 >> --- a/drivers/thermal/samsung/exynos_tmu.h >> +++ b/drivers/thermal/samsung/exynos_tmu.h >> @@ -34,6 +34,7 @@ enum soc_type { >> SOC_ARCH_EXYNOS5420, >> SOC_ARCH_EXYNOS5420_TRIMINFO, >> SOC_ARCH_EXYNOS5440, >> + SOC_ARCH_EXYNOS7, >> }; >> >> /** > > I think that the code is OK, despite some minor comments. > > Thanks Abhilash ! Thanks much for the review. I will re-spin addressing your comments soon. Regards, Abhilash -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |