Re: [PATCH v12 1/2] thermal: loongson-2: add thermal management support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2023/2/8 下午6:49, Daniel Lezcano 写道:
On Wed, Feb 08, 2023 at 11:13:33AM +0800, zhuyinbo wrote:

[ ... ]

+struct loongson2_thermal_data {
+	struct thermal_zone_device *tzd;
'tzd' won't be needed after taking into account the comments
The 'tzd' element is needed,  because the thermal_zone_device_update need
pass a data->tzd element.

static irqreturn_t loongson2_thermal_irq_thread(int irq, void *dev)
{
         struct loongson2_thermal_data *data = dev;

         thermal_zone_device_update(data->tzd,
                                    THERMAL_EVENT_UNSPECIFIED);
         enable_irq(data->irq);

         return IRQ_HANDLED;
}
After taking into account all the comments, enabled_irq() won't be
called, so 'data' won't be needed. 'tzd' will be passed to
devm_request_threaded_irq() instead of 'data'.

As loongson2_thermal_irq_thread() is the only place where 'tzd' is
needed and 'tzd' being local to the call site of
thermal_zone_device_register() and devm_request_threaded_irq(), there
is no need to store the pointer in the 'data' structure.

okay, I got it. I will remove tzd element in data struct.


+	int irq;
'irq' won't be needed after taking into account the comments
I will drop it.
+	int id;
+	void __iomem *regs;
+	struct platform_device *pdev;
'pdev' is not needed
I will drop it.
+	u16 ctrl_low_val;
+	u16 ctrl_hi_val;
Those fields won't be needed after taking into account the comments
I will drop it.
+};
+
+static int loongson2_thermal_set(struct loongson2_thermal_data *data,
+					int low, int high, bool enable)
+{
+	u64 reg_ctrl = 0;
+	int reg_off = data->id * 2;
+
+	if (low > high)
+		return -EINVAL;
+
+	low = max(low, -100);
+	high = min(high, 155);
Documentation says -40, 125
My previous calculation is to consider that the range of 8bit representation is 0 to 255, and node (cpu) temp=Thens0_ out -100, So the temperature range is 0-100 ~ 255-100, and this range includes -40~125.  In fact, the range described in the manual is - 40~125, I will
adop it.

+	low += 100;
+	high += 100;
Why are those values added to the low and high ? Did you mean low += 0x100 ?

Mind to describe a bit the register layout?
node(cpu) temp = Thens0_out -100,

low and high is record node temp, so low and high need add '100' as
Thens0_out.
If I refer to the documentation it is a raw value converted from
centigrade. The function has degree.

So it should be:

temp_deci = temp_milli / 10

raw = temp_to_raw(temp_deci);

-> temp_to_raw to be determined from temp = (raw * 731) / 0x4000 - 273

[ ... ]
I have review the 3a5000 datasheet,  what you said is right about 3a5000, but in 2k1000 datasheet,

the calculation of temperature is follows:

Temperature = Thens0_out - 100
get_temp return value is (Temperature * 1000) and set_trips is (value /1000) in 2k1000.
I don't find a caculate about "temp_deci = temp_milli / 10" . Are you talking about "
temp_deci = temp_milli / 1000" in set_trips?


+static int loongson2_thermal_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	u32 reg_val;
+	struct loongson2_thermal_data *data = tz->devdata;
+
+	reg_val = readl(data->regs + LOONGSON2_TSENSOR_OUT);
+	*temp = ((reg_val & 0xff) - 100) * 1000;
Why '-100' ?

Is the unit returned 'degrees' ?
node(cpu) temp = Thens0_out -100,

Here we need to get a node temp.
If I refer to the Loongson-3A5000 manual and assuming it is the right
one, the documentation says:

Temperature = Thens0_out * 731 / 0x4000 - 273

The unit is centigrade.

[ ... ]

Yes, 3a5000 is what you said, but 2k1000 is calculated as follows:

Temperature = Thens0_out - 100

+	writeb(0xff, data->regs + LOONGSON2_TSENSOR_STATUS);
+
+	loongson2_thermal_set(data, 0, 0, false);
It would be nicer to use a reset line if it is available
sorry, I don't get your meaning. Please describe more details about 'reset
line'.
After a reset, the thermal controller should be in default state and the interrupt
flag cleared.

One example:

[1] https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/arch/arm64/boot/dts/nvidia/tegra210.dtsi#n1560
[2] https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/tegra/soctherm.c#n2169

Then search in the driver for:
      reset_control_assert(reset);
      reset_control_deassert(reset);


[ ... ]

Thanks

   -- Daniel
thanks your explicate!   but our platform doesn't support it.





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux