| Message ID | 20231128005849.19044-6-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for H616 Thermal system | expand |
Hi Martin, Andre. May I inquire? Why do we need a separate sun50i_h616_ths_calibrate() function? Why can't we just extend an existing sun50i_h6_ths_calibrate()? At my glance the calculations in both functions are the same. We just need to handle a special case for the 4th sensor. Best regards, Maksim
On Sat, 9 Dec 2023 13:44:34 +0300 Maksim Kiselev <bigunclemax@gmail.com> wrote: Hi Maksim, > Hi Martin, Andre. > > May I inquire? Why do we need a separate sun50i_h616_ths_calibrate() > function? Why can't we just extend an existing sun50i_h6_ths_calibrate()? > > At my glance the calculations in both functions are the same. We just > need to handle a special case for the 4th sensor. You seem to be right, they are indeed the same, just written slightly differently. Do you already have any patches that unify that? I don't know if Martin or I find time to do it this week, but we could also optimise this later. Cheers, Andre
пн, 11 дек. 2023 г. в 02:48, Andre Przywara <andre.przywara@arm.com>: > > On Sat, 9 Dec 2023 13:44:34 +0300 > Maksim Kiselev <bigunclemax@gmail.com> wrote: > > Hi Maksim, > > > Hi Martin, Andre. > > > > May I inquire? Why do we need a separate sun50i_h616_ths_calibrate() > > function? Why can't we just extend an existing sun50i_h6_ths_calibrate()? > > > > At my glance the calculations in both functions are the same. We just > > need to handle a special case for the 4th sensor. > > You seem to be right, they are indeed the same, just written slightly > differently. Do you already have any patches that unify that? No, I don't have any patches for that yet. But I can do it if you told me where to send the patch. Should I put it here as a reply? > I don't know if Martin or I find time to do it this week, but we could > also optimise this later. > > Cheers, > Andre Best regards, Maksim
On Tue, 12 Dec 2023 21:09:45 +0300 Maxim Kiselev <bigunclemax@gmail.com> wrote: Hi Maksim, sorry for the delay. > пн, 11 дек. 2023 г. в 02:48, Andre Przywara <andre.przywara@arm.com>: > > > > On Sat, 9 Dec 2023 13:44:34 +0300 > > Maksim Kiselev <bigunclemax@gmail.com> wrote: > > > > Hi Maksim, > > > > > Hi Martin, Andre. > > > > > > May I inquire? Why do we need a separate sun50i_h616_ths_calibrate() > > > function? Why can't we just extend an existing sun50i_h6_ths_calibrate()? > > > > > > At my glance the calculations in both functions are the same. We just > > > need to handle a special case for the 4th sensor. > > > > You seem to be right, they are indeed the same, just written slightly > > differently. Do you already have any patches that unify that? > > No, I don't have any patches for that yet. But I can do it if you told > me where to > send the patch. Should I put it here as a reply? Can you make one patch on top of mainline, that prepares the existing sun50i_h6_ths_calibrate() function to deal with four sensors? I would then include this patch of yours in the next submission, and put Martin's H616 patch (now probably just the struct ths_thermal_chip) on top then. Please send this patch just to the list (CC:ing people like Martin and me), starting a new thread. Many thanks, Andre > > I don't know if Martin or I find time to do it this week, but we could > > also optimise this later. > > > > Cheers, > > Andre > > Best regards, > Maksim
Hi Andre, чт, 14 дек. 2023 г. в 13:59, Andre Przywara <andre.przywara@arm.com>: > > On Tue, 12 Dec 2023 21:09:45 +0300 > Maxim Kiselev <bigunclemax@gmail.com> wrote: > > Hi Maksim, > > sorry for the delay. Now it's my turn to apologize for the delay :) > > > пн, 11 дек. 2023 г. в 02:48, Andre Przywara <andre.przywara@arm.com>: > > > > > > On Sat, 9 Dec 2023 13:44:34 +0300 > > > Maksim Kiselev <bigunclemax@gmail.com> wrote: > > > > > > Hi Maksim, > > > > > > > Hi Martin, Andre. > > > > > > > > May I inquire? Why do we need a separate sun50i_h616_ths_calibrate() > > > > function? Why can't we just extend an existing sun50i_h6_ths_calibrate()? > > > > > > > > At my glance the calculations in both functions are the same. We just > > > > need to handle a special case for the 4th sensor. > > > > > > You seem to be right, they are indeed the same, just written slightly > > > differently. Do you already have any patches that unify that? > > > > No, I don't have any patches for that yet. But I can do it if you told > > me where to > > send the patch. Should I put it here as a reply? > > Can you make one patch on top of mainline, that prepares the existing > sun50i_h6_ths_calibrate() function to deal with four sensors? I would then > include this patch of yours in the next submission, and put Martin's H616 > patch (now probably just the struct ths_thermal_chip) on top then. > > Please send this patch just to the list (CC:ing people like Martin and > me), starting a new thread. You can find my patch here: https://lore.kernel.org/all/20231217133637.54773-1-bigunclemax@gmail.com/ Best regards, Maksim > Many thanks, > Andre > > > > I don't know if Martin or I find time to do it this week, but we could > > > also optimise this later. > > > > > > Cheers, > > > Andre > > > > Best regards, > > Maksim >
diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c index 920e419ce7343..9a404fa9d76a9 100644 --- a/drivers/thermal/sun8i_thermal.c +++ b/drivers/thermal/sun8i_thermal.c @@ -280,6 +280,64 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev, return 0; } +static int sun50i_h616_ths_calibrate(struct ths_device *tmdev, + u16 *caldata, int callen) +{ + struct device *dev = tmdev->dev; + int i, ft_temp; + + if (!caldata[0]) + return -EINVAL; + + /* + * h616 efuse THS calibration data layout: + * + * 0 11 16 27 32 43 48 57 + * +----------+-----------+-----------+-----------+ + * | temp | |sensor0| |sensor1| |sensor2| | + * +----------+-----------+-----------+-----------+ + * ^ ^ ^ + * | | | + * | | sensor3[11:8] + * | sensor3[7:4] + * sensor3[3:0] + * + * The calibration data on the H616 is the ambient temperature and + * sensor values that are filled during the factory test stage. + * + * The unit of stored FT temperature is 0.1 degree celsius. + */ + ft_temp = caldata[0] & FT_TEMP_MASK; + + for (i = 0; i < tmdev->chip->sensor_num; i++) { + int delta, cdata, offset, reg, temp; + + if (i == 3) + reg = (caldata[1] >> 12) + | ((caldata[2] >> 12) << 4) + | ((caldata[3] >> 12) << 8); + else + reg = (int)caldata[i + 1] & TEMP_CALIB_MASK; + + temp = tmdev->chip->calc_temp(tmdev, i, reg); + delta = ((temp - ft_temp * 100) * 10) / tmdev->chip->scale; + cdata = CALIBRATE_DEFAULT - delta; + if (cdata & ~TEMP_CALIB_MASK) { + dev_warn(dev, "sensor%d is not calibrated.\n", i); + + continue; + } + + offset = (i % 2) * 16; + regmap_update_bits(tmdev->regmap, + SUN50I_H6_THS_TEMP_CALIB + (i / 2 * 4), + TEMP_CALIB_MASK << offset, + cdata << offset); + } + + return 0; +} + static int sun8i_ths_calibrate(struct ths_device *tmdev) { struct nvmem_cell *calcell; @@ -659,6 +717,20 @@ static const struct ths_thermal_chip sun50i_h6_ths = { .calc_temp = sun8i_ths_calc_temp, }; +static const struct ths_thermal_chip sun50i_h616_ths = { + .sensor_num = 4, + .has_bus_clk_reset = true, + .needs_syscon = true, + .ft_deviation = 8000, + .offset = 263655, + .scale = 810, + .temp_data_base = SUN50I_H6_THS_TEMP_DATA, + .calibrate = sun50i_h616_ths_calibrate, + .init = sun50i_h6_thermal_init, + .irq_ack = sun50i_h6_irq_ack, + .calc_temp = sun8i_ths_calc_temp, +}; + static const struct of_device_id of_ths_match[] = { { .compatible = "allwinner,sun8i-a83t-ths", .data = &sun8i_a83t_ths }, { .compatible = "allwinner,sun8i-h3-ths", .data = &sun8i_h3_ths }, @@ -667,6 +739,7 @@ static const struct of_device_id of_ths_match[] = { { .compatible = "allwinner,sun50i-a100-ths", .data = &sun50i_a100_ths }, { .compatible = "allwinner,sun50i-h5-ths", .data = &sun50i_h5_ths }, { .compatible = "allwinner,sun50i-h6-ths", .data = &sun50i_h6_ths }, + { .compatible = "allwinner,sun50i-h616-ths", .data = &sun50i_h616_ths }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, of_ths_match);