diff mbox series

[v1,2/2] hwmon: (it87) Add entries to dmi_table

Message ID 20230103064612.404401-3-frank@crawford.emu.id.au (mailing list archive)
State Changes Requested
Headers show
Series hwmon: (it87) Set second Super-IO chip in configuration mode | expand

Commit Message

Frank Crawford Jan. 3, 2023, 6:46 a.m. UTC 
Call initialisation of second chipset.

Update the dmi_table with mother boards that have been tested.

Signed-off-by: Frank Crawford <frank@crawford.emu.id.au>
---
 drivers/hwmon/it87.c | 58 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Comments

Guenter Roeck Jan. 3, 2023, 6:46 p.m. UTC | #1 
On Tue, Jan 03, 2023 at 05:46:12PM +1100, Frank Crawford wrote:
> Call initialisation of second chipset.
> 
> Update the dmi_table with mother boards that have been tested.
> 

That doesn't really describe what is done and why. See
Documentation/process/submitting-patches.rst, "Describe your changes".
The comment iin the code is much better. 

> Signed-off-by: Frank Crawford <frank@crawford.emu.id.au>
> ---
>  drivers/hwmon/it87.c | 58 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> index 4ebce2c661d7..2ecfa2c901f6 100644
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> @@ -3315,6 +3315,27 @@ static int it87_dmi_cb(const struct dmi_system_id *dmi_entry)
>  	return 1;
>  }
>  
> +/*
> + * On various Gigabyte AM4 boards (AB350, AX370), the second Super-IO chip
> + * (IT8792E) needs to be in configuration mode before accessing the first
> + * due to a bug in IT8792E which otherwise results in LPC bus access errors.
> + * This needs to be done before accessing the first Super-IO chip since
> + * the second chip may have been accessed prior to loading this driver.
> + *
> + * The problem is also reported to affect IT8795E, which is used on X299 boards
> + * and has the same chip ID as IT8792E (0x8733). It also appears to affect
> + * systems with IT8790E, which is used on some Z97X-Gaming boards as well as
> + * Z87X-OC.
> + * DMI entries for those systems will be added as they become available and
> + * as the problem is confirmed to affect those boards.
> + */
> +static int gigabyte_sio2_force(const struct dmi_system_id *dmi_entry)

s/gigabyte/it87/

While this is seen on Gigabyte boards, it doesn't _have_ to be Gigabyte
specific. Other boards i using the same chips (it there are any) may be
affected as well.

> +{
> +	__superio_enter(REG_4E);
> +
> +	return it87_dmi_cb(dmi_entry);
> +};
> +
>  /*
>   * On the Shuttle SN68PT, FAN_CTL2 is apparently not
>   * connected to a fan, but to something else. One user
> @@ -3337,7 +3358,44 @@ static struct it87_dmi_data nvidia_fn68pt = {
>  		.driver_data = data, \
>  	}
>  
> +#define IT87_DMI_MATCH_GBT(name, cb, data) \
> +	IT87_DMI_MATCH_VND("Gigabyte Technology Co., Ltd.", name, cb, data)
> +
>  static const struct dmi_system_id it87_dmi_table[] __initconst = {
> +	IT87_DMI_MATCH_GBT("AB350", gigabyte_sio2_force, NULL),
> +		/* ? + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("AX370", gigabyte_sio2_force, NULL),
> +		/* ? + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("Z97X-Gaming G1", gigabyte_sio2_force, NULL),
> +		/* ? + IT8790E */
> +	IT87_DMI_MATCH_GBT("TRX40 AORUS XTREME", gigabyte_sio2_force, NULL),
> +		/* IT8688E + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("Z390 AORUS ULTRA-CF", gigabyte_sio2_force, NULL),
> +		/* IT8688E + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("Z490 AORUS ELITE AC", it87_dmi_cb, NULL),
> +		/* IT8688E */

Why list boards with only a single chip ?

> +	IT87_DMI_MATCH_GBT("B550 AORUS PRO AC", gigabyte_sio2_force, NULL),
> +		/* IT8688E + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("B560I AORUS PRO AX", it87_dmi_cb, NULL),
> +		/* IT8689E */
> +	IT87_DMI_MATCH_GBT("X570 AORUS ELITE WIFI", it87_dmi_cb, NULL),
> +		/* IT8688E */
> +	IT87_DMI_MATCH_GBT("X570 AORUS MASTER", gigabyte_sio2_force, NULL),
> +		/* IT8688E + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("X570 AORUS PRO", gigabyte_sio2_force, NULL),
> +		/* IT8688E + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("X570 AORUS PRO WIFI", gigabyte_sio2_force, NULL),
> +		/* IT8688E + IT8792E/IT8795E */
> +	IT87_DMI_MATCH_GBT("X570 I AORUS PRO WIFI", it87_dmi_cb, NULL),
> +		/* IT8688E */
> +	IT87_DMI_MATCH_GBT("X570S AERO G", gigabyte_sio2_force, NULL),
> +		/* IT8689E + IT87952E */

IT87952E ? Is that chip known to be affected as well ?

> +	IT87_DMI_MATCH_GBT("X670E AORUS MASTER", it87_dmi_cb, NULL),
> +		/* IT8689E - Note there may also be a second chip */

Don't add an entry unless known that it is needed.

> +	IT87_DMI_MATCH_GBT("Z690 AORUS PRO DDR4", gigabyte_sio2_force, NULL),
> +		/* IT8689E + IT87952E */
> +	IT87_DMI_MATCH_GBT("Z690 AORUS PRO", gigabyte_sio2_force, NULL),
> +		/* IT8689E + IT87952E */
>  	IT87_DMI_MATCH_VND("nVIDIA", "FN68PT", it87_dmi_cb, &nvidia_fn68pt),
>  	{ }
>  
> -- 
> 2.38.1
>
Frank Crawford Jan. 4, 2023, 12:43 a.m. UTC | #2 
On Tue, 2023-01-03 at 10:46 -0800, Guenter Roeck wrote:
> On Tue, Jan 03, 2023 at 05:46:12PM +1100, Frank Crawford wrote:
> > Call initialisation of second chipset.
> > 
> > Update the dmi_table with mother boards that have been tested.
> > 
> 
> That doesn't really describe what is done and why. See
> Documentation/process/submitting-patches.rst, "Describe your
> changes".
> The comment iin the code is much better. 

Will fix up the descriptions.

> > Signed-off-by: Frank Crawford <frank@crawford.emu.id.au>
> > ---
> >  drivers/hwmon/it87.c | 58
> > ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> > index 4ebce2c661d7..2ecfa2c901f6 100644
> > --- a/drivers/hwmon/it87.c
> > +++ b/drivers/hwmon/it87.c
> > @@ -3315,6 +3315,27 @@ static int it87_dmi_cb(const struct
> > dmi_system_id *dmi_entry)
> >         return 1;
> >  }
> >  
> > +/*
> > + * On various Gigabyte AM4 boards (AB350, AX370), the second
> > Super-IO chip
> > + * (IT8792E) needs to be in configuration mode before accessing
> > the first
> > + * due to a bug in IT8792E which otherwise results in LPC bus
> > access errors.
> > + * This needs to be done before accessing the first Super-IO chip
> > since
> > + * the second chip may have been accessed prior to loading this
> > driver.
> > + *
> > + * The problem is also reported to affect IT8795E, which is used
> > on X299 boards
> > + * and has the same chip ID as IT8792E (0x8733). It also appears
> > to affect
> > + * systems with IT8790E, which is used on some Z97X-Gaming boards
> > as well as
> > + * Z87X-OC.
> > + * DMI entries for those systems will be added as they become
> > available and
> > + * as the problem is confirmed to affect those boards.
> > + */
> > +static int gigabyte_sio2_force(const struct dmi_system_id
> > *dmi_entry)
> 
> s/gigabyte/it87/
> 
> While this is seen on Gigabyte boards, it doesn't _have_ to be
> Gigabyte
> specific. Other boards i using the same chips (it there are any) may
> be
> affected as well.

Will update it.
> 
> > +{
> > +       __superio_enter(REG_4E);
> > +
> > +       return it87_dmi_cb(dmi_entry);
> > +};
> > +
> >  /*
> >   * On the Shuttle SN68PT, FAN_CTL2 is apparently not
> >   * connected to a fan, but to something else. One user
> > @@ -3337,7 +3358,44 @@ static struct it87_dmi_data nvidia_fn68pt =
> > {
> >                 .driver_data = data, \
> >         }
> >  
> > +#define IT87_DMI_MATCH_GBT(name, cb, data) \
> > +       IT87_DMI_MATCH_VND("Gigabyte Technology Co., Ltd.", name,
> > cb, data)
> > +
> >  static const struct dmi_system_id it87_dmi_table[] __initconst = {
> > +       IT87_DMI_MATCH_GBT("AB350", gigabyte_sio2_force, NULL),
> > +               /* ? + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("AX370", gigabyte_sio2_force, NULL),
> > +               /* ? + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("Z97X-Gaming G1", gigabyte_sio2_force,
> > NULL),
> > +               /* ? + IT8790E */
> > +       IT87_DMI_MATCH_GBT("TRX40 AORUS XTREME",
> > gigabyte_sio2_force, NULL),
> > +               /* IT8688E + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("Z390 AORUS ULTRA-CF",
> > gigabyte_sio2_force, NULL),
> > +               /* IT8688E + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("Z490 AORUS ELITE AC", it87_dmi_cb,
> > NULL),
> > +               /* IT8688E */
> 
> Why list boards with only a single chip ?

Because I'm stupid and just copied and pasted from my other code.

I'll drop out the single chips and resubmit.
> 
> > +       IT87_DMI_MATCH_GBT("B550 AORUS PRO AC",
> > gigabyte_sio2_force, NULL),
> > +               /* IT8688E + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("B560I AORUS PRO AX", it87_dmi_cb,
> > NULL),
> > +               /* IT8689E */
> > +       IT87_DMI_MATCH_GBT("X570 AORUS ELITE WIFI", it87_dmi_cb,
> > NULL),
> > +               /* IT8688E */
> > +       IT87_DMI_MATCH_GBT("X570 AORUS MASTER",
> > gigabyte_sio2_force, NULL),
> > +               /* IT8688E + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("X570 AORUS PRO", gigabyte_sio2_force,
> > NULL),
> > +               /* IT8688E + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("X570 AORUS PRO WIFI",
> > gigabyte_sio2_force, NULL),
> > +               /* IT8688E + IT8792E/IT8795E */
> > +       IT87_DMI_MATCH_GBT("X570 I AORUS PRO WIFI", it87_dmi_cb,
> > NULL),
> > +               /* IT8688E */
> > +       IT87_DMI_MATCH_GBT("X570S AERO G", gigabyte_sio2_force,
> > NULL),
> > +               /* IT8689E + IT87952E */
> 
> IT87952E ? Is that chip known to be affected as well ?
> 
> > +       IT87_DMI_MATCH_GBT("X670E AORUS MASTER", it87_dmi_cb,
> > NULL),
> > +               /* IT8689E - Note there may also be a second chip
> > */
> 
> Don't add an entry unless known that it is needed.

That is more a note for future testing, but for now will drop this
board as it is currently treated as a single chip board.
> 
> > +       IT87_DMI_MATCH_GBT("Z690 AORUS PRO DDR4",
> > gigabyte_sio2_force, NULL),
> > +               /* IT8689E + IT87952E */
> > +       IT87_DMI_MATCH_GBT("Z690 AORUS PRO", gigabyte_sio2_force,
> > NULL),
> > +               /* IT8689E + IT87952E */
> >         IT87_DMI_MATCH_VND("nVIDIA", "FN68PT", it87_dmi_cb,
> > &nvidia_fn68pt),
> >         { }
> >  
> > -- 
> > 2.38.1

Regards
Frank
>
diff mbox series

Patch

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 4ebce2c661d7..2ecfa2c901f6 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -3315,6 +3315,27 @@  static int it87_dmi_cb(const struct dmi_system_id *dmi_entry)
 	return 1;
 }
 
+/*
+ * On various Gigabyte AM4 boards (AB350, AX370), the second Super-IO chip
+ * (IT8792E) needs to be in configuration mode before accessing the first
+ * due to a bug in IT8792E which otherwise results in LPC bus access errors.
+ * This needs to be done before accessing the first Super-IO chip since
+ * the second chip may have been accessed prior to loading this driver.
+ *
+ * The problem is also reported to affect IT8795E, which is used on X299 boards
+ * and has the same chip ID as IT8792E (0x8733). It also appears to affect
+ * systems with IT8790E, which is used on some Z97X-Gaming boards as well as
+ * Z87X-OC.
+ * DMI entries for those systems will be added as they become available and
+ * as the problem is confirmed to affect those boards.
+ */
+static int gigabyte_sio2_force(const struct dmi_system_id *dmi_entry)
+{
+	__superio_enter(REG_4E);
+
+	return it87_dmi_cb(dmi_entry);
+};
+
 /*
  * On the Shuttle SN68PT, FAN_CTL2 is apparently not
  * connected to a fan, but to something else. One user
@@ -3337,7 +3358,44 @@  static struct it87_dmi_data nvidia_fn68pt = {
 		.driver_data = data, \
 	}
 
+#define IT87_DMI_MATCH_GBT(name, cb, data) \
+	IT87_DMI_MATCH_VND("Gigabyte Technology Co., Ltd.", name, cb, data)
+
 static const struct dmi_system_id it87_dmi_table[] __initconst = {
+	IT87_DMI_MATCH_GBT("AB350", gigabyte_sio2_force, NULL),
+		/* ? + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("AX370", gigabyte_sio2_force, NULL),
+		/* ? + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("Z97X-Gaming G1", gigabyte_sio2_force, NULL),
+		/* ? + IT8790E */
+	IT87_DMI_MATCH_GBT("TRX40 AORUS XTREME", gigabyte_sio2_force, NULL),
+		/* IT8688E + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("Z390 AORUS ULTRA-CF", gigabyte_sio2_force, NULL),
+		/* IT8688E + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("Z490 AORUS ELITE AC", it87_dmi_cb, NULL),
+		/* IT8688E */
+	IT87_DMI_MATCH_GBT("B550 AORUS PRO AC", gigabyte_sio2_force, NULL),
+		/* IT8688E + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("B560I AORUS PRO AX", it87_dmi_cb, NULL),
+		/* IT8689E */
+	IT87_DMI_MATCH_GBT("X570 AORUS ELITE WIFI", it87_dmi_cb, NULL),
+		/* IT8688E */
+	IT87_DMI_MATCH_GBT("X570 AORUS MASTER", gigabyte_sio2_force, NULL),
+		/* IT8688E + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("X570 AORUS PRO", gigabyte_sio2_force, NULL),
+		/* IT8688E + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("X570 AORUS PRO WIFI", gigabyte_sio2_force, NULL),
+		/* IT8688E + IT8792E/IT8795E */
+	IT87_DMI_MATCH_GBT("X570 I AORUS PRO WIFI", it87_dmi_cb, NULL),
+		/* IT8688E */
+	IT87_DMI_MATCH_GBT("X570S AERO G", gigabyte_sio2_force, NULL),
+		/* IT8689E + IT87952E */
+	IT87_DMI_MATCH_GBT("X670E AORUS MASTER", it87_dmi_cb, NULL),
+		/* IT8689E - Note there may also be a second chip */
+	IT87_DMI_MATCH_GBT("Z690 AORUS PRO DDR4", gigabyte_sio2_force, NULL),
+		/* IT8689E + IT87952E */
+	IT87_DMI_MATCH_GBT("Z690 AORUS PRO", gigabyte_sio2_force, NULL),
+		/* IT8689E + IT87952E */
 	IT87_DMI_MATCH_VND("nVIDIA", "FN68PT", it87_dmi_cb, &nvidia_fn68pt),
 	{ }