[PATCH] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[PATCH] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Ellie Hermaszewska]

Only the temp sensors that I can verify are present. T_Sensor is the
temperature reading of a 10kΩ β=3435K NTC thermistor optionally
connected to the T_SENSOR header.

Signed-off-by: Ellie Hermaszewska <kernel@monoid.al>
---
 drivers/hwmon/asus-ec-sensors.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
index 51f9c2db403e..36f9e38000d5 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -244,6 +244,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
 	EC_SENSOR("Motherboard", hwmon_temp, 1, 0x00, 0x32),
 	[ec_sensor_temp_vrm] =
 		EC_SENSOR("VRM", hwmon_temp, 1, 0x00, 0x33),
+	[ec_sensor_temp_t_sensor] =
+		EC_SENSOR("T_Sensor", hwmon_temp, 1, 0x00, 0x36),
 	[ec_sensor_temp_water_in] =
 		EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
 	[ec_sensor_temp_water_out] =
@@ -344,6 +346,14 @@ static const struct ec_board_info board_info_crosshair_x670e_hero = {
 	.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 |
+		SENSOR_TEMP_MB | SENSOR_TEMP_VRM,
+	.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 |
@@ -490,6 +500,8 @@ static const struct dmi_system_id dmi_table[] = {
 					&board_info_crosshair_viii_hero),
 	DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR X670E HERO",
 					&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("ROG MAXIMUS XI HERO",
 					&board_info_maximus_xi_hero),
 	DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO (WI-FI)",
-- 
2.42.0

Re: [PATCH] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Eugene Shalygin]

Hi,

Thank you for submitting the patch! I don't understand how does your
note that only the T_Sensor presence can be verified correlate with
SENSOR_TEMP_CPU | SENSOR_TEMP_CPU_PACKAGE |
SENSOR_TEMP_MB | SENSOR_TEMP_VRM enabled. Could you clarify, please?

Based on the EC registers dump you provided [1], I believe it is safe
to enable Water_In and Water_Oout sensors as well.

And please add the board name to list in Documentation/hwmon/asus_ec_sensors.rst

Cheers,
Eugene

[1] https://github.com/zeule/asus-ec-sensors/issues/42#issuecomment-1742062260

Re: [PATCH] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Eugene Shalygin]

Oh, sorry, I misread the board name. Please scratch the bits about water temps.

Cheers,
Eugene

On Thu, 19 Oct 2023 at 11:43, Eugene Shalygin <eugene.shalygin@gmail.com> wrote:
>
> Hi,
>
> Thank you for submitting the patch! I don't understand how does your
> note that only the T_Sensor presence can be verified correlate with
> SENSOR_TEMP_CPU | SENSOR_TEMP_CPU_PACKAGE |
> SENSOR_TEMP_MB | SENSOR_TEMP_VRM enabled. Could you clarify, please?
>
> Based on the EC registers dump you provided [1], I believe it is safe
> to enable Water_In and Water_Oout sensors as well.
>
> And please add the board name to list in Documentation/hwmon/asus_ec_sensors.rst
>
> Cheers,
> Eugene
>
> [1] https://github.com/zeule/asus-ec-sensors/issues/42#issuecomment-1742062260

[PATCH v2] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Ellie Hermaszewska]

These are two separate statements, describing the set of sensors
implemented (those I could verify) and the specifics of the T_Sensor new
to the X670E motherboards.

Best wishes,
Ellie

Only the temp sensors that I can verify are present.

T_Sensor is the temperature reading of a 10kΩ β=3435K NTC thermistor
optionally connected to the T_SENSOR header.

The other sensors are as found on the X670E HERO.

Signed-off-by: Ellie Hermaszewska <kernel@monoid.al>
---
 Documentation/hwmon/asus_ec_sensors.rst |  1 +
 drivers/hwmon/asus-ec-sensors.c         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
index 7e3cd5b6686f..0bf99ba406dd 100644
--- a/Documentation/hwmon/asus_ec_sensors.rst
+++ b/Documentation/hwmon/asus_ec_sensors.rst
@@ -15,6 +15,7 @@ Supported boards:
  * ROG CROSSHAIR VIII HERO
  * ROG CROSSHAIR VIII IMPACT
  * ROG CROSSHAIR X670E HERO
+ * ROG CROSSHAIR X670E GENE
  * 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 51f9c2db403e..36f9e38000d5 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -244,6 +244,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
 	EC_SENSOR("Motherboard", hwmon_temp, 1, 0x00, 0x32),
 	[ec_sensor_temp_vrm] =
 		EC_SENSOR("VRM", hwmon_temp, 1, 0x00, 0x33),
+	[ec_sensor_temp_t_sensor] =
+		EC_SENSOR("T_Sensor", hwmon_temp, 1, 0x00, 0x36),
 	[ec_sensor_temp_water_in] =
 		EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
 	[ec_sensor_temp_water_out] =
@@ -344,6 +346,14 @@ static const struct ec_board_info board_info_crosshair_x670e_hero = {
 	.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 |
+		SENSOR_TEMP_MB | SENSOR_TEMP_VRM,
+	.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 |
@@ -490,6 +500,8 @@ static const struct dmi_system_id dmi_table[] = {
 					&board_info_crosshair_viii_hero),
 	DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR X670E HERO",
 					&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("ROG MAXIMUS XI HERO",
 					&board_info_maximus_xi_hero),
 	DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO (WI-FI)",
-- 
2.42.0

Re: [PATCH v2] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Guenter Roeck]

On Thu, Oct 19, 2023 at 09:51:58PM +0800, Ellie Hermaszewska wrote:
> These are two separate statements, describing the set of sensors
> implemented (those I could verify) and the specifics of the T_Sensor new
> to the X670E motherboards.
> 
> Best wishes,
> Ellie
> 
> Only the temp sensors that I can verify are present.
> 
> T_Sensor is the temperature reading of a 10kΩ β=3435K NTC thermistor
> optionally connected to the T_SENSOR header.
> 
> The other sensors are as found on the X670E HERO.
> 

This is not an acceptable commit description.

Guenter

Re: Re: [PATCH v2] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Ellie Hermaszewska]

On 10/26/23 03:35, Guenter Roeck wrote:
 > This is not an acceptable commit description.

This is not acceptable feedback.

I am unable to accept it because it is not clear to me what you think
should be changed.

Is it because I misplaced the message to Eugene? Is it because of the
Greek characters? Is is not descriptive enough of the change, or in the
wrong tense, or has incorrect punctuation? Do I need to include my
testing methodology?

If it's only something minor, then please also feel free to correct it
yourself before applying. If you can't or it's not something minor,
then please let me know what ought to change and I can try to correct it.

If you don't let me know, then I will have to guess and possibly waste 
everyone's time further.

Ellie

Re: [PATCH v2] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Guenter Roeck]

On 10/25/23 21:46, Ellie Hermaszewska wrote:
> On 10/26/23 03:35, Guenter Roeck wrote:
>  > This is not an acceptable commit description.
> 
> This is not acceptable feedback.
> 
> I am unable to accept it because it is not clear to me what you think
> should be changed.
> 
> Is it because I misplaced the message to Eugene? Is it because of the
> Greek characters? Is is not descriptive enough of the change, or in the
> wrong tense, or has incorrect punctuation? Do I need to include my
> testing methodology?
> 
> If it's only something minor, then please also feel free to correct it
> yourself before applying. If you can't or it's not something minor,
> then please let me know what ought to change and I can try to correct it.
> 
> If you don't let me know, then I will have to guess and possibly waste everyone's time further.
> 

Please consider reading and following the directions in
Documentation/process/submitting-patches.rst.

Thanks,
Guenter

Re: [PATCH v2] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Ellie Hermaszewska]

On 10/26/23 13:01, Guenter Roeck wrote:
> On 10/25/23 21:46, Ellie Hermaszewska wrote:
>> On 10/26/23 03:35, Guenter Roeck wrote:
>>  > This is not an acceptable commit description.
>>
>> This is not acceptable feedback.
>>
>> I am unable to accept it because it is not clear to me what you think
>> should be changed.
>>
>> Is it because I misplaced the message to Eugene? Is it because of the
>> Greek characters? Is is not descriptive enough of the change, or in the
>> wrong tense, or has incorrect punctuation? Do I need to include my
>> testing methodology?
>>
>> If it's only something minor, then please also feel free to correct it
>> yourself before applying. If you can't or it's not something minor,
>> then please let me know what ought to change and I can try to correct it.
>>
>> If you don't let me know, then I will have to guess and possibly waste 
>> everyone's time further.
>>
> 
> Please consider reading and following the directions in
> Documentation/process/submitting-patches.rst.

I will guess that it was my misplaced reply, and submit again without 
that part.

Thank you for your time.

[PATCH v3] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Ellie Hermaszewska]

Only the temp sensors that I can verify are present.

T_Sensor is the temperature reading of a 10kΩ β=3435K NTC thermistor
optionally connected to the T_SENSOR header.

The other sensors are as found on the X670E Hero.

Signed-off-by: Ellie Hermaszewska <kernel@monoid.al>
---
 Documentation/hwmon/asus_ec_sensors.rst |  1 +
 drivers/hwmon/asus-ec-sensors.c         | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/Documentation/hwmon/asus_ec_sensors.rst b/Documentation/hwmon/asus_ec_sensors.rst
index 7e3cd5b6686f..0bf99ba406dd 100644
--- a/Documentation/hwmon/asus_ec_sensors.rst
+++ b/Documentation/hwmon/asus_ec_sensors.rst
@@ -15,6 +15,7 @@ Supported boards:
  * ROG CROSSHAIR VIII HERO
  * ROG CROSSHAIR VIII IMPACT
  * ROG CROSSHAIR X670E HERO
+ * ROG CROSSHAIR X670E GENE
  * 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 51f9c2db403e..36f9e38000d5 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -244,6 +244,8 @@ static const struct ec_sensor_info sensors_family_amd_600[] = {
 	EC_SENSOR("Motherboard", hwmon_temp, 1, 0x00, 0x32),
 	[ec_sensor_temp_vrm] =
 		EC_SENSOR("VRM", hwmon_temp, 1, 0x00, 0x33),
+	[ec_sensor_temp_t_sensor] =
+		EC_SENSOR("T_Sensor", hwmon_temp, 1, 0x00, 0x36),
 	[ec_sensor_temp_water_in] =
 		EC_SENSOR("Water_In", hwmon_temp, 1, 0x01, 0x00),
 	[ec_sensor_temp_water_out] =
@@ -344,6 +346,14 @@ static const struct ec_board_info board_info_crosshair_x670e_hero = {
 	.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 |
+		SENSOR_TEMP_MB | SENSOR_TEMP_VRM,
+	.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 |
@@ -490,6 +500,8 @@ static const struct dmi_system_id dmi_table[] = {
 					&board_info_crosshair_viii_hero),
 	DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG CROSSHAIR X670E HERO",
 					&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("ROG MAXIMUS XI HERO",
 					&board_info_maximus_xi_hero),
 	DMI_EXACT_MATCH_ASUS_BOARD_NAME("ROG MAXIMUS XI HERO (WI-FI)",
-- 
2.42.0

Re: [PATCH v3] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Guenter Roeck]

On Thu, Oct 26, 2023 at 06:43:22PM +0800, Ellie Hermaszewska wrote:
> Only the temp sensors that I can verify are present.
> 
> T_Sensor is the temperature reading of a 10kΩ β=3435K NTC thermistor
> optionally connected to the T_SENSOR header.
> 
> The other sensors are as found on the X670E Hero.
> 
> Signed-off-by: Ellie Hermaszewska <kernel@monoid.al>

Applied, but please provide changelogs in the future.

Guenter

Re: [PATCH v2] hwmon: (asus-ec-sensors) add ROG Crosshair X670E Gene.

[Guenter Roeck]

On 10/26/23 03:41, Ellie Hermaszewska wrote:
> On 10/26/23 13:01, Guenter Roeck wrote:
>> On 10/25/23 21:46, Ellie Hermaszewska wrote:
>>> On 10/26/23 03:35, Guenter Roeck wrote:
>>>  > This is not an acceptable commit description.
>>>
>>> This is not acceptable feedback.
>>>
>>> I am unable to accept it because it is not clear to me what you think
>>> should be changed.
>>>
>>> Is it because I misplaced the message to Eugene? Is it because of the
>>> Greek characters? Is is not descriptive enough of the change, or in the
>>> wrong tense, or has incorrect punctuation? Do I need to include my
>>> testing methodology?
>>>
>>> If it's only something minor, then please also feel free to correct it
>>> yourself before applying. If you can't or it's not something minor,
>>> then please let me know what ought to change and I can try to correct it.
>>>
>>> If you don't let me know, then I will have to guess and possibly waste everyone's time further.
>>>
>>
>> Please consider reading and following the directions in
>> Documentation/process/submitting-patches.rst.
> 
> I will guess that it was my misplaced reply, and submit again without that part.
> 

 From the document:

 > Other comments relevant only to the moment or the maintainer, not
 > suitable for the permanent changelog, should also go here. A good
 > example of such comments might be ``patch changelogs`` which describe
 > what has changed between the v1 and v2 version of the patch.

 > Please put this information **after** the ``---`` line which separates
 > the changelog from the rest of the patch. The version information is
 > not part of the changelog which gets committed to the git tree. It is
 > additional information for the reviewers. If it's placed above the
 > commit tags, it needs manual interaction to remove it. If it is below
 > the separator line, it gets automatically stripped off when applying the
 > patch::
 > ...
 > [ ... ] When sending a next
 > version, add a ``patch changelog`` to the cover letter or to individual patches
 > explaining difference against previous submission

Keeping the patch description clean does not mean to _drop_ the changelog
or additional information not intended to be added to the commit.

Guenter