[PATCH 0/1] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[PATCH 0/1] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Li XingYang]

this is my mainboard:
dmidecode | grep -A3 "Base Board"
Base Board Information
        Manufacturer: ASUSTeK COMPUTER INC.
        Product Name: TUF GAMING X670E-PLUS
        Version: Rev 1.xx

i use the version of BIOS:
TUF-GAMING-X670E-PLUS-ASUS-3042

asus-ec-sensors show when use this patch:
sensors 'asusec-*'
asusec-isa-0000
Adapter: ISA adapter
CPU_Opt:      914 RPM
CPU:          +39.0°C
CPU Package:  +48.0°C
Motherboard:  +31.0°C
VRM:          +46.0°C
Water_In:     +35.0°C
Water_Out:    +33.0°C


Li XingYang (1):
  hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

 Documentation/hwmon/asus_ec_sensors.rst |  1 +
 drivers/hwmon/asus-ec-sensors.c         | 13 +++++++++++++
 2 files changed, 14 insertions(+)

-- 
2.47.1

[PATCH 1/1] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Li XingYang]

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..d9b4074080bd 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[] = {
@@ -354,6 +356,15 @@ static const struct ec_board_info board_info_crosshair_x670e_hero = {
 	.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_x670e_gene = {
 	.sensors = SENSOR_TEMP_CPU | SENSOR_TEMP_CPU_PACKAGE |
 		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.1

Re: [PATCH 1/1] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Eugene Shalygin]

Hi,

Thank you for the patch! One little request, please, is to keep the
definitions list sorted, that is, could you, please, put the new board
definition in the source file after ROG CROSSHAIR X670E GENE?
Otherwise LGTM.

Kind regards,
Eugene

On Sat, 30 Nov 2024 at 14:38, Li XingYang <yanhuoguifan@gmail.com> wrote:
>
> 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..d9b4074080bd 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[] = {
> @@ -354,6 +356,15 @@ static const struct ec_board_info board_info_crosshair_x670e_hero = {
>         .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_x670e_gene = {
>         .sensors = SENSOR_TEMP_CPU | SENSOR_TEMP_CPU_PACKAGE |
>                 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.1
>

[PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Li XingYang]

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.1

Re: [PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Guenter Roeck]

On 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)",

Re: [PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Eugene Shalygin]

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

Re: [PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Guenter Roeck]

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

Re: [PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Li XingYang]

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.

Re: [PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Guenter Roeck]

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

Re: [PATCH v2] hwmon: (asus-ec-sensors) add TUF GAMING X670E PLUS

[Eugene Shalygin]

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