Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Deepin-Kernel-SIG] [linux-6.6.y] [deepin] hwmon: (k10temp) Check return value of hygon_read_temp() #411

Merged

Conversation

Avenger-285714
Copy link
Collaborator

Check the return value of amd_smn_read() before saving a value. This ensures invalid values aren't saved or used.

There are three cases here with slightly different behavior.

  1. read_tempreg_nb_zen():
    This is a function pointer which does not include a return code.
    In this case, set the register value to 0 on failure. This
    enforces Read-as-Zero behavior.

  2. k10temp_read_temp():
    This function does have return codes, so return the error code
    from the failed register read. Continued operation is not
    necessary, since there is no valid data from the register.
    Furthermore, if the register value was set to 0, then the
    following operation would underflow.

  3. k10temp_get_ccd_support():
    This function reads the same register from multiple CCD
    instances in a loop. And a bitmask is formed if a specific bit
    is set in each register instance. The loop should continue on a
    failed register read, skipping the bit check.

WangYuli:
When check amd_smn_read(), don't forget Hygon.

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.11-rc6&id=c2d79cc5455c891de6c93e1e0c73d806e299c54f
Link: d693d4a

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from avenger-285714. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Avenger-285714 Avenger-285714 changed the title [Deepin-Kernel-SIG] [linux-6.6.y] [deepin] hwmon: (k10temp) Check return value of hygon_read_temp() [WIP] [Deepin-Kernel-SIG] [linux-6.6.y] [deepin] hwmon: (k10temp) Check return value of hygon_read_temp() Sep 8, 2024
@Avenger-285714 Avenger-285714 changed the title [WIP] [Deepin-Kernel-SIG] [linux-6.6.y] [deepin] hwmon: (k10temp) Check return value of hygon_read_temp() [Deepin-Kernel-SIG] [linux-6.6.y] [deepin] hwmon: (k10temp) Check return value of hygon_read_temp() Sep 8, 2024
@Avenger-285714 Avenger-285714 force-pushed the linux-6.6.y branch 3 times, most recently from de98c30 to aaeb2c2 Compare September 11, 2024 08:57
Avenger-285714 and others added 3 commits September 11, 2024 17:07
Check the return value of amd_smn_read() before saving a value. This
ensures invalid values aren't saved or used.

There are three cases here with slightly different behavior.

1) read_tempreg_nb_zen():
	This is a function pointer which does not include a return code.
	In this case, set the register value to 0 on failure. This
	enforces Read-as-Zero behavior.

2) k10temp_read_temp():
	This function does have return codes, so return the error code
	from the failed register read. Continued operation is not
	necessary, since there is no valid data from the register.
	Furthermore, if the register value was set to 0, then the
	following operation would underflow.

3) k10temp_get_ccd_support():
	This function reads the same register from multiple CCD
	instances in a loop. And a bitmask is formed if a specific bit
	is set in each register instance. The loop should continue on a
	failed register read, skipping the bit check.

WangYuli:
  When check amd_smn_read(), don't forget Hygon.
  Thanks for code review from Hygon.

Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.11-rc6&id=c2d79cc5455c891de6c93e1e0c73d806e299c54f
Link: deepin-community@d693d4a
Signed-off-by: WangYuli <[email protected]>
…cess kernel memory

If we have a buggy bios,and we have no bios update for it,
we cannot access the _OSC map,
so the CPPC cpufreq driver will not probed in some case.
example: Kunpeng 920  8 core Desktop Board

Log:
ACPI CPPC: Failed to ioremap PCC comm region mem for 0
ACPI CPPC: Failed to find PCC channel for subspace 0
ACPI CPPC: Failed to find PCC channel for subspace 0
ACPI CPPC: Failed to find PCC channel for subspace 0
ACPI CPPC: Failed to find PCC channel for subspace 0
ACPI CPPC: Failed to find PCC channel for subspace 0
ACPI CPPC: Failed to find PCC channel for subspace 0
ACPI CPPC: Failed to find PCC channel for subspace 0

Link:https://gitee.com/openeuler/kernel/issues/I39AN0
Signed-off-by: Wentao Guan <[email protected]>
If we have a buggy bios,and we have no bios update for it,
we cannot acked CPPCv2.
so the CPPC cpufreq driver will not probed in some case.
example: Kunpeng 920  8 core Desktop Board

Test result: cpu freq can from 200Mhz to 2.6Ghz.

Log:
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
cppc_acpi:acpi_cppc_processor_probe: ACPI CPPC: CPPC v2 _OSC not acked
@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • hygon_read_temp函数中,返回值ret被错误地赋值给参数regval,应该将返回值赋给函数的返回值。
  • hygon_read_temp函数中的ret变量应该声明为int类型,而不是u32类型,以匹配其返回值类型。
  • hygon_read_temp函数中的ret变量缺少注释说明其用途,应该添加注释以提高代码可读性。
  • k10temp_read_temp函数中的ret变量在if条件判断后没有使用,应该移除以避免混淆。
  • k10temp_read_temp函数中的ret变量缺少注释说明其用途,应该添加注释以提高代码可读性。
  • k10temp_get_ccd_support_2nd函数中的ret变量在循环中被重新声明,应该使用不同的变量名以避免混淆。
  • k10temp_get_ccd_support_2nd函数中的ret变量缺少注释说明其用途,应该添加注释以提高代码可读性。
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用regval
  • k10temp_get_ccd_support_2nd函数中,对regval的访问应该使用&regval来获取其只读地址,而不是直接使用

@Avenger-285714 Avenger-285714 merged commit e11dfa1 into deepin-community:linux-6.6.y Sep 11, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants