Documentation/hwmon/asus_ec_sensors.rst | 1 + drivers/hwmon/asus-ec-sensors.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)
add asus-ec-sensors on the mainboard TUF GAMING X670E PLUS
support these sensors:
SENSOR_TEMP_CPU, SENSOR_TEMP_CPU_PACKAGE, SENSOR_TEMP_MB
SENSOR_TEMP_VRM, SENSOR_TEMP_WATER_IN, SENSOR_TEMP_WATER_OUT
and SENSOR_FAN_CPU_OPT
Signed-off-by: Li XingYang <yanhuoguifan@gmail.com>
---
Documentation/hwmon/asus_ec_sensors.rst | 1 +
drivers/hwmon/asus-ec-sensors.c | 13 +++++++++++++
2 files changed, 14 insertions(+)
diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
index ca38922f4ec5..d049a62719b0 100644
--- a/Documentation/hwmon/asus_ec_sensors.rst
+++ b/Documentation/hwmon/asus_ec_sensors.rst
@@ -17,6 +17,7 @@ Supported boards:
* ROG CROSSHAIR VIII IMPACT
* ROG CROSSHAIR X670E HERO
* ROG CROSSHAIR X670E GENE
+ * TUF GAMING X670E PLUS
* ROG MAXIMUS XI HERO
* ROG MAXIMUS XI HERO (WI-FI)
* ROG STRIX B550-E GAMING
diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
index 9555366aeaf0..f02e4f5cc6db 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -250,6 +250,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
[ec_sensor_temp_water_out] =
EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x01),
+ [ec_sensor_fan_cpu_opt] =
+ EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0),
};
static const struct ec_sensor_info sensors_family_intel_300[] = {
@@ -362,6 +364,15 @@ static const struct ec_board_info board_info_crosshair_x670e_gene = {
.family = family_amd_600_series,
};
+static const struct ec_board_info board_info_tuf_gaming_x670e_plus = {
+ .sensors = SENSOR_TEMP_CPU | SENSOR_TEMP_CPU_PACKAGE |
+ SENSOR_TEMP_MB | SENSOR_TEMP_VRM |
+ SENSOR_TEMP_WATER_IN | SENSOR_TEMP_WATER_OUT |
+ SENSOR_FAN_CPU_OPT,
+ .mutex_path = ACPI_GLOBAL_LOCK_PSEUDO_PATH,
+ .family = family_amd_600_series,
+};
+
static const struct ec_board_info board_info_crosshair_viii_dark_hero = {
.sensors = SENSOR_SET_TEMP_CHIPSET_CPU_MB |
SENSOR_TEMP_T_SENSOR |
@@ -512,6 +523,8 @@ static const struct dmi_system_id dmi_table[] = {
&board_info_crosshair_x670e_hero),
DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR X670E GENE",
&board_info_crosshair_x670e_gene),
+ DMI_EXACT_MATCH_ASUS_BOARD_NAME("TUF GAMING X670E-PLUS",
+ &board_info_tuf_gaming_x670e_plus),
DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO",
&board_info_maximus_xi_hero),
DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO (WI-FI)",
--
2.47.1On 11/30/24 06:47, Li XingYang wrote:
> add asus-ec-sensors on the mainboard TUF GAMING X670E PLUS
as/add/Add/
Same for subject.
> support these sensors:
> SENSOR_TEMP_CPU, SENSOR_TEMP_CPU_PACKAGE, SENSOR_TEMP_MB
> SENSOR_TEMP_VRM, SENSOR_TEMP_WATER_IN, SENSOR_TEMP_WATER_OUT
> and SENSOR_FAN_CPU_OPT
>
The individual sensors supported by this board are irrelevant for the
patch description.
> Signed-off-by: Li XingYang <yanhuoguifan@gmail.com>
Please do not send new revisions of a patch as response of an older
series, and please always provide a change log.
> ---
> Documentation/hwmon/asus_ec_sensors.rst | 1 +
> drivers/hwmon/asus-ec-sensors.c | 13 +++++++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
> index ca38922f4ec5..d049a62719b0 100644
> --- a/Documentation/hwmon/asus_ec_sensors.rst
> +++ b/Documentation/hwmon/asus_ec_sensors.rst
> @@ -17,6 +17,7 @@ Supported boards:
> * ROG CROSSHAIR VIII IMPACT
> * ROG CROSSHAIR X670E HERO
> * ROG CROSSHAIR X670E GENE
> + * TUF GAMING X670E PLUS
> * ROG MAXIMUS XI HERO
> * ROG MAXIMUS XI HERO (WI-FI)
> * ROG STRIX B550-E GAMING
I don't understand how this is "sorted". What is the sorting criteria ?
> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index 9555366aeaf0..f02e4f5cc6db 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
> @@ -250,6 +250,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
> EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
> [ec_sensor_temp_water_out] =
> EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x01),
> + [ec_sensor_fan_cpu_opt] =
> + EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0),
This is an unrelated change. It affects other boards of the same family.
It needs to be a separate patch, it needs to be explained, and it needs to
get some confirmation that it works on the other boards of the same series.
Thanks,
Guenter
> };
>
> static const struct ec_sensor_info sensors_family_intel_300[] = {
> @@ -362,6 +364,15 @@ static const struct ec_board_info board_info_crosshair_x670e_gene = {
> .family = family_amd_600_series,
> };
>
> +static const struct ec_board_info board_info_tuf_gaming_x670e_plus = {
> + .sensors = SENSOR_TEMP_CPU | SENSOR_TEMP_CPU_PACKAGE |
> + SENSOR_TEMP_MB | SENSOR_TEMP_VRM |
> + SENSOR_TEMP_WATER_IN | SENSOR_TEMP_WATER_OUT |
> + SENSOR_FAN_CPU_OPT,
> + .mutex_path = ACPI_GLOBAL_LOCK_PSEUDO_PATH,
> + .family = family_amd_600_series,
> +};
> +
> static const struct ec_board_info board_info_crosshair_viii_dark_hero = {
> .sensors = SENSOR_SET_TEMP_CHIPSET_CPU_MB |
> SENSOR_TEMP_T_SENSOR |
> @@ -512,6 +523,8 @@ static const struct dmi_system_id dmi_table[] = {
> &board_info_crosshair_x670e_hero),
> DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR X670E GENE",
> &board_info_crosshair_x670e_gene),
> + DMI_EXACT_MATCH_ASUS_BOARD_NAME("TUF GAMING X670E-PLUS",
> + &board_info_tuf_gaming_x670e_plus),
> DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO",
> &board_info_maximus_xi_hero),
> DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO (WI-FI)",
On Sat, Nov 30, 2024 at 07:29:21AM -0800, Guenter Roeck wrote:
> > support these sensors:
> > SENSOR_TEMP_CPU, SENSOR_TEMP_CPU_PACKAGE, SENSOR_TEMP_MB
> > SENSOR_TEMP_VRM, SENSOR_TEMP_WATER_IN, SENSOR_TEMP_WATER_OUT
> > and SENSOR_FAN_CPU_OPT
> >
>
> The individual sensors supported by this board are irrelevant for the
> patch description.
>
My original intention was to describe the sensors supported by this new motherboard.
If you think it's irrelevant, I can delete the descriptions of these sensors.
> > Signed-off-by: Li XingYang <yanhuoguifan@gmail.com>
>
> Please do not send new revisions of a patch as response of an older
> series, and please always provide a change log.
>
Sorry, I cannot fully understand this meaning.
Should I use the new version of the patch to reply to the old version of
the patch instead of responding to the questions raised
> > ---
> > Documentation/hwmon/asus_ec_sensors.rst | 1 +
> > drivers/hwmon/asus-ec-sensors.c | 13 +++++++++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
> > index ca38922f4ec5..d049a62719b0 100644
> > --- a/Documentation/hwmon/asus_ec_sensors.rst
> > +++ b/Documentation/hwmon/asus_ec_sensors.rst
> > @@ -17,6 +17,7 @@ Supported boards:
> > * ROG CROSSHAIR VIII IMPACT
> > * ROG CROSSHAIR X670E HERO
> > * ROG CROSSHAIR X670E GENE
> > + * TUF GAMING X670E PLUS
> > * ROG MAXIMUS XI HERO
> > * ROG MAXIMUS XI HERO (WI-FI)
> > * ROG STRIX B550-E GAMING
>
> I don't understand how this is "sorted". What is the sorting criteria ?
>
>
At first, I saw that the ROG CROSSHAIR X670E GENE was the
last motherboard in the x670e series on this list,
so I placed the new x670e motherboard after it.
Now I realize that this list originally followed alphabetical order,
perhaps it would be better for me to put the new motherboard first,
as well as the source files first
> > diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> > index 9555366aeaf0..f02e4f5cc6db 100644
> > --- a/drivers/hwmon/asus-ec-sensors.c
> > +++ b/drivers/hwmon/asus-ec-sensors.c
> > @@ -250,6 +250,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
> > EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
> > [ec_sensor_temp_water_out] =
> > EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x01),
> > + [ec_sensor_fan_cpu_opt] =
> > + EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0),
>
> This is an unrelated change. It affects other boards of the same family.
> It needs to be a separate patch, it needs to be explained, and it needs to
> get some confirmation that it works on the other boards of the same series.
>
> Thanks,
> Guenter
I found that in the LibreHardwareMonitor project,
the registers used by Amd600 to operate FanCPUOpt are described:
https://github.com/LibreHardwareMonitor/LibreHardwareMonitor/blob/master/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs
BoardFamily.Amd600, new Dictionary<ECSensor, EmbeddedControllerSource>
{
{ ECSensor.FanCPUOpt, new EmbeddedControllerSource("CPU Optional Fan", SensorType.Fan, 0x00b0, 2) },
}
I think this is suitable for the AMD 600 motherboard, and it does work on my motherboard as well.
Of course, I cannot guarantee that all ASUS AMD600 motherboards can use this interface,
In fact, the ec_sensor_temp_t_sensor originally defined in sensors_family_amd_600
is also not applicable on my motherboard.
I think sensors_family_amd_600 only provides a common interface,
and the specific motherboard selection still needs to be tested.
I will dismantle this part into a separate patch.
On 11/30/24 17:17, Li XingYang wrote:
[ ... ]
>> Please do not send new revisions of a patch as response of an older
>> series, and please always provide a change log.
>>
> Sorry, I cannot fully understand this meaning.
> Should I use the new version of the patch to reply to the old version of
> the patch instead of responding to the questions raised
If you send new revisions of a patch or patch series as reply to older
versions of that patch series, it may get lost because it is not identified
as updated patch but as reply to an older patch or patch series. Also see
"Explicit In-Reply-To headers" in Documentation/process/submitting-patches.rst
[ ... ]
>> This is an unrelated change. It affects other boards of the same family.
>> It needs to be a separate patch, it needs to be explained, and it needs to
>> get some confirmation that it works on the other boards of the same series.
>>
>> Thanks,
>> Guenter
> I found that in the LibreHardwareMonitor project,
> the registers used by Amd600 to operate FanCPUOpt are described:
> https://github.com/LibreHardwareMonitor/LibreHardwareMonitor/blob/master/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs
> BoardFamily.Amd600, new Dictionary<ECSensor, EmbeddedControllerSource>
> {
> { ECSensor.FanCPUOpt, new EmbeddedControllerSource("CPU Optional Fan", SensorType.Fan, 0x00b0, 2) },
> }
>
> I think this is suitable for the AMD 600 motherboard, and it does work on my motherboard as well.
That makes sense, but it is still unrelated to this patch and, worse,
not even mentioned in the patch description. See "Separate your changes"
in Documentation/process/submitting-patches.rst.
Guenter
Hi,
> >> This is an unrelated change. It affects other boards of the same family.
> >> It needs to be a separate patch, it needs to be explained, and it needs to
> >> get some confirmation that it works on the other boards of the same series.
> > I found that in the LibreHardwareMonitor project,
> > the registers used by Amd600 to operate FanCPUOpt are described:
> > https://github.com/LibreHardwareMonitor/LibreHardwareMonitor/blob/master/LibreHardwareMonitorLib/Hardware/Motherboard/Lpc/EC/EmbeddedController.cs
> > BoardFamily.Amd600, new Dictionary<ECSensor, EmbeddedControllerSource>
> > {
> > { ECSensor.FanCPUOpt, new EmbeddedControllerSource("CPU Optional Fan", SensorType.Fan, 0x00b0, 2) },
> > }
> >
> > I think this is suitable for the AMD 600 motherboard, and it does work on my motherboard as well.
>
> That makes sense, but it is still unrelated to this patch and, worse,
> not even mentioned in the patch description. See "Separate your changes"
> in Documentation/process/submitting-patches.rst.
Can confirm CPU_Opt is still at 0xb0 for ProArt X870E Creator Wifi, so
it is the same in AMD families 500 to 800. The introduction of the
sensor to the 600th family should not affect other boards, because
they do not use it yet,
Kind regards,
Eugene
Hi Günter,
> > diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
> > index ca38922f4ec5..d049a62719b0 100644
> > --- a/Documentation/hwmon/asus_ec_sensors.rst
> > +++ b/Documentation/hwmon/asus_ec_sensors.rst
> > @@ -17,6 +17,7 @@ Supported boards:
> > * ROG CROSSHAIR VIII IMPACT
> > * ROG CROSSHAIR X670E HERO
> > * ROG CROSSHAIR X670E GENE
> > + * TUF GAMING X670E PLUS
> > * ROG MAXIMUS XI HERO
> > * ROG MAXIMUS XI HERO (WI-FI)
> > * ROG STRIX B550-E GAMING
>
> I don't understand how this is "sorted". What is the sorting criteria ?
I believe the list in static const struct dmi_system_id dmi_table[]
and the list in the .rst file are in the same order, and I want the
board declarations to follow that.
> > diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> > index 9555366aeaf0..f02e4f5cc6db 100644
> > --- a/drivers/hwmon/asus-ec-sensors.c
> > +++ b/drivers/hwmon/asus-ec-sensors.c
> > @@ -250,6 +250,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
> > EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
> > [ec_sensor_temp_water_out] =
> > EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x01),
> > + [ec_sensor_fan_cpu_opt] =
> > + EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0),
>
> This is an unrelated change. It affects other boards of the same family.
> It needs to be a separate patch, it needs to be explained, and it needs to
> get some confirmation that it works on the other boards of the same series.
Well, it is the same register as in the previous generation, and while
it would be nice to confirm that it works in other models of the 600th
family, I can't see how XingYang can do that. I can check with the AMD
800th series though...
Kind regards,
Eugene
On 11/30/24 07:47, Eugene Shalygin wrote:
> Hi Günter,
>
>>> diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
>>> index ca38922f4ec5..d049a62719b0 100644
>>> --- a/Documentation/hwmon/asus_ec_sensors.rst
>>> +++ b/Documentation/hwmon/asus_ec_sensors.rst
>>> @@ -17,6 +17,7 @@ Supported boards:
>>> * ROG CROSSHAIR VIII IMPACT
>>> * ROG CROSSHAIR X670E HERO
>>> * ROG CROSSHAIR X670E GENE
>>> + * TUF GAMING X670E PLUS
>>> * ROG MAXIMUS XI HERO
>>> * ROG MAXIMUS XI HERO (WI-FI)
>>> * ROG STRIX B550-E GAMING
>>
>> I don't understand how this is "sorted". What is the sorting criteria ?
>
> I believe the list in static const struct dmi_system_id dmi_table[]
> and the list in the .rst file are in the same order, and I want the
> board declarations to follow that.
>
So you don't care about alphabetic order, just about using the same order
in both files ? Fine with me, and I don't have to understand it, but it is a
deviation from the current model and should be documented for reference to
ensure that I don't call out people for not using non-alphabetic order
in the future. If there is some other order, it would be even more important
to document it to help people understand what it is supposed to be.
>>> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
>>> index 9555366aeaf0..f02e4f5cc6db 100644
>>> --- a/drivers/hwmon/asus-ec-sensors.c
>>> +++ b/drivers/hwmon/asus-ec-sensors.c
>>> @@ -250,6 +250,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
>>> EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
>>> [ec_sensor_temp_water_out] =
>>> EC_SENSOR("Water_Out", hwmon_temp, 1, 0x01, 0x01),
>>> + [ec_sensor_fan_cpu_opt] =
>>> + EC_SENSOR("CPU_Opt", hwmon_fan, 2, 0x00, 0xb0),
>>
>> This is an unrelated change. It affects other boards of the same family.
>> It needs to be a separate patch, it needs to be explained, and it needs to
>> get some confirmation that it works on the other boards of the same series.
>
> Well, it is the same register as in the previous generation, and while
> it would be nice to confirm that it works in other models of the 600th
> family, I can't see how XingYang can do that. I can check with the AMD
> 800th series though...
>
Ok with me if you confirm it, but it still needs to be a separate patch
since it it not about adding support for a specific board.
Guenter
© 2016 - 2026 Red Hat, Inc.