On Wed, 2024-04-10 at 08:10 -0700, Guenter Roeck wrote:
> On Mon, Apr 01, 2024 at 01:56:03PM +1100, Frank Crawford wrote:
> > Rename previous definitions to match the new information that they
> > are
> > preinitialised by the BIOS and should not receive codes to enter or
> > exit
> > configuration mode.
> >
>
> That is wrong. NOEXIT is needed for broken chips where two chips are
> on the
> sio bus, and disabling sio access on the broken chip results in sio
> access
> errors. The description is also wrong, because SIO mode still needs
> to be
> _entered_.
As noted in the patch group write up, this change has come from the
technical specifications for the chips not for the board. If by SIO
mode you mean the sequence "0x87,0x01,0x55,0xAA" then it should not be
used for these chips according to people with access to the
specification documents.
Unfortunately, I don't have direct access to these documents, so cannot
quote the full description.
Now, the macro name may not be the best (BIOSOPEN), and I'm happy to
change it to something better, but the current name of "NOEXIT" is also
wrong.
However, for the chips that this relates to, and are defined to use in
the it87_devices structure, you can access the chip details without the
the superio_enter sequence, as that is specifically the read that
occurs to find the chipID, and I have tested it on a number of
different chips, both of this type and older ones that do need the
entry sequence.
What makes this a little more difficult is that the chips that it
affects also only ever appears to be the second chip on the bus, which
may be by design, or just current usage.
I will add that the use of enter sequence has been confirmed to fail
and cause the exact chip lockup concerned about on the Gigabyte X670E
Aorus Master board. While you may say that we should only do this for
that board, the information I have received is that it is cause by
incorrect access to those chipIDs, not board specific.
>
> Also, a BIOS open mode, if it indeed exists, would have to be be
> board
> specific, not chip specific.
Now here my description may be wrong in it being BIOS related, but
rather it is specifically the chip initialisation, but the details on
access came from the chip specifications.
>
> Guenter
Regards
Frank
>
> > Signed-off-by: Frank Crawford <frank@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/hwmon/it87.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
> > index fbe86cec6055..6eeba3a01e3c 100644
> > --- a/drivers/hwmon/it87.c
> > +++ b/drivers/hwmon/it87.c
> > @@ -320,7 +320,7 @@ struct it87_devices {
> > * second SIO address. Never exit configuration mode on these
> > * chips to avoid the problem.
> > */
> > -#define FEAT_CONF_NOEXIT BIT(19) /* Chip should not exit conf mode
> > */
> > +#define FEAT_CONF_BIOSOPEN BIT(19) /* Chip conf mode enabled by
> > BIOS */
> > #define FEAT_FOUR_FANS BIT(20) /* Supports four fans */
> > #define FEAT_FOUR_PWM BIT(21) /* Supports four fan controls */
> > #define FEAT_FOUR_TEMP BIT(22)
> > @@ -452,7 +452,7 @@ static const struct it87_devices it87_devices[]
> > = {
> > .model = "IT8790E",
> > .features = FEAT_NEWER_AUTOPWM | FEAT_12MV_ADC | FEAT_16BIT_FANS
> > | FEAT_TEMP_OFFSET | FEAT_TEMP_PECI | FEAT_IN7_INTERNAL
> > - | FEAT_PWM_FREQ2 | FEAT_FANCTL_ONOFF | FEAT_CONF_NOEXIT,
> > + | FEAT_PWM_FREQ2 | FEAT_FANCTL_ONOFF | FEAT_CONF_BIOSOPEN,
> > .peci_mask = 0x07,
> > },
> > [it8792] = {
> > @@ -461,7 +461,7 @@ static const struct it87_devices it87_devices[]
> > = {
> > .features = FEAT_NEWER_AUTOPWM | FEAT_16BIT_FANS
> > | FEAT_TEMP_OFFSET | FEAT_TEMP_OLD_PECI | FEAT_TEMP_PECI
> > | FEAT_10_9MV_ADC | FEAT_IN7_INTERNAL | FEAT_FANCTL_ONOFF
> > - | FEAT_CONF_NOEXIT,
> > + | FEAT_CONF_BIOSOPEN,
> > .peci_mask = 0x07,
> > .old_peci_mask = 0x02, /* Actually reports PCH */
> > },
> > @@ -507,7 +507,7 @@ static const struct it87_devices it87_devices[]
> > = {
> > .features = FEAT_NEWER_AUTOPWM | FEAT_16BIT_FANS
> > | FEAT_TEMP_OFFSET | FEAT_TEMP_OLD_PECI | FEAT_TEMP_PECI
> > | FEAT_10_9MV_ADC | FEAT_IN7_INTERNAL | FEAT_FANCTL_ONOFF
> > - | FEAT_CONF_NOEXIT,
> > + | FEAT_CONF_BIOSOPEN,
> > .peci_mask = 0x07,
> > .old_peci_mask = 0x02, /* Actually reports PCH */
> > },
> > @@ -544,7 +544,7 @@ static const struct it87_devices it87_devices[]
> > = {
> > #define has_four_temp(data) ((data)->features & FEAT_FOUR_TEMP)
> > #define has_six_temp(data) ((data)->features & FEAT_SIX_TEMP)
> > #define has_vin3_5v(data) ((data)->features & FEAT_VIN3_5V)
> > -#define has_conf_noexit(data) ((data)->features &
> > FEAT_CONF_NOEXIT)
> > +#define has_conf_biosopen(data) ((data)->features &
> > FEAT_CONF_BIOSOPEN)
> > #define has_scaling(data) ((data)->features & (FEAT_12MV_ADC | \
> > FEAT_10_9MV_ADC))
> > #define has_fanctl_onoff(data) ((data)->features &
> > FEAT_FANCTL_ONOFF)
> > @@ -748,7 +748,7 @@ static int smbus_disable(struct it87_data
> > *data)
> > superio_select(data->sioaddr, PME);
> > superio_outb(data->sioaddr, IT87_SPECIAL_CFG_REG,
> > data->ec_special_config & ~data->smbus_bitmap);
> > - superio_exit(data->sioaddr, has_conf_noexit(data));
> > + superio_exit(data->sioaddr, has_conf_biosopen(data));
> > }
> > return 0;
> > }
> > @@ -765,7 +765,7 @@ static int smbus_enable(struct it87_data *data)
> > superio_select(data->sioaddr, PME);
> > superio_outb(data->sioaddr, IT87_SPECIAL_CFG_REG,
> > data->ec_special_config);
> > - superio_exit(data->sioaddr, has_conf_noexit(data));
> > + superio_exit(data->sioaddr, has_conf_biosopen(data));
> > }
> > return 0;
> > }
> > @@ -3143,7 +3143,7 @@ static int __init it87_find(int sioaddr,
> > unsigned short *address,
> > }
> >
> > exit:
> > - superio_exit(sioaddr, config ? has_conf_noexit(config) : false);
> > + superio_exit(sioaddr, config ? has_conf_biosopen(config) :
> > false);
> > return err;
> > }
> >
> > @@ -3540,7 +3540,7 @@ static void it87_resume_sio(struct
> > platform_device *pdev)
> > reg2c);
> > }
> >
> > - superio_exit(data->sioaddr, has_conf_noexit(data));
> > + superio_exit(data->sioaddr, has_conf_biosopen(data));
> > }
> >
> > static int it87_resume(struct device *dev)
