Re: adt7460 temperature target

I'm not saying your issue doesn't need to be addressed.

However, your diff is affecting every single machine of all types with
that chip.  Some of these i2c chips are way more complicated than the
driver suggests, and we don't know if there is another subsystem which
expects them to not be reprogrammed.

Nuno Vasconcellos <[email protected]> wrote:

> I have a PowerMac6,4 (eMac) and noticed its fan starts to run as soon as =
the machine is powered up and it keeps running as long as it stays powered
> on.
>=20
> I'm not fluent in openfirmware but fussing there, I had an impression tha=
t there could be a way there...
>=20
> Alternatively, maybe in Jon's machine case it's worth to check the follow=
ing project:
> https://github.com/andreafabrizi/G4FanControl
>=20
> Cheers.
>=20
> Le samedi 25 novembre 2023 =C3=A0 08:57 -0700, Theo de Raadt a =C3=A9crit=
=C2=A0:
> > For the temperature sensing drivers we have *intentionally* avoided doi=
ng
> > writes to i2c devices.
> >=20
> > On many machines, ACPI/SMI retain control of these devices, and make
> > changes behind OpenBSD's back without us knowing what is going on.=C2=
=A0 We
> > don't know if this will break a machine, do to their changes depending
> > up on a specific configuration.=C2=A0 The breakage could be subtle and =
very
> > weird.=C2=A0 This is a policy we established for all these drivers, and=
 we've
> > avoided creating too many exceptions to it because if the exception
> > becomes the rule, subtle interactions could create harm.
> >=20
> > As a result, all sensor drivers for these chips only do 'sensing',
> > meaning they only do reads.=C2=A0 (OK, a few drivers do writes to "enab=
le"
> > them, if we discover that the system has not yet enabled them, then it
> > means there is no BIOS driving behind our back; this situation feels
> > more safe, then OpenBSD is in control).
> >=20
> > That said, your argument is for macppc.=C2=A0 I would not argue against=
 a
> > macppc-only configuration change, especially if it is clearly from a
> > "chip not configured state"), but I don't know what that code would look
> > like, whether it is an #ifdef or run-time comparison of the machine
> > architecture and model.
> >=20
> > [email protected]=C2=A0wrote:
> >=20
> > > Hello. It's been about 6 months since I tweaked adt(4) to control
> > > the temperature target that the cooling system uses to regulate
> > > itself, following the source code of G4FanControl which may be found
> > > around online.
> > >=20
> > > This makes it possible to power the fans on earlier and thus improve
> > > thermals and, hopefully, lifespan of the system which -given these
> > > macppc machines are quite old and increasingly rare- I would argue
> > > is worth preserving. I myself find it quite comfortable at 46 degC
> > >=20
> > > Perhaps it would be better to add a sysctl to control the temperature
> > > target? This diff is a fully functional proof of concept I guess,
> > > would really appreciate your thoughts.
> > >=20
> > > All the best
> > >=20
> > > Index: dev/i2c/adt7460.c
> > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > > RCS file: /cvs/src/sys/dev/i2c/adt7460.c,v
> > > retrieving revision 1.22
> > > diff -u -p -r1.22 adt7460.c
> > > --- dev/i2c/adt7460.c=C2=A0=C2=A0=C2=A06 Apr 2022 18:59:28 -0000=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A01.22
> > > +++ dev/i2c/adt7460.c=C2=A0=C2=A0=C2=A023 Nov 2023 16:23:48 -0000
> > > @@ -34,6 +34,9 @@
> > > =C2=A0#define ADT7460_REM1_TEMP=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A00x=
25
> > > =C2=A0#define ADT7460_LOCAL_TEMP=C2=A0=C2=A0=C2=A0=C2=A0=C2=A00x26
> > > =C2=A0#define ADT7460_REM2_TEMP=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A00x=
27
> > > +#define ADT7460_REM1_TEMP_TRG=C2=A0=C2=A00x68
> > > +#define ADT7460_LOCAL_TEMP_TRG=C2=A00x67
> > > +#define ADT7460_REM2_TEMP_TRG=C2=A0=C2=A00x69
> > > =C2=A0#define ADT7460_TACH1L=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00x28
> > > =C2=A0#define ADT7460_TACH1H=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00x29
> > > =C2=A0#define ADT7460_TACH2L=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A00x2a
> > > @@ -193,6 +196,16 @@ adt_attach(struct device *parent, struct
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sc->sc_addr, &cmd, sizeof cmd=
, &data, sizeof data, 0)) {
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
iic_release_bus(sc->sc_tag, 0);
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
printf(": cannot set control register\n");
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ret=
urn;
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0}
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > +
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0for (cmd =3D ADT7460_LOCAL=
_TEMP_TRG; cmd <=3D ADT7460_REM2_TEMP_TRG; cmd++){
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0data =3D 46; /* temperature target in deg C*/
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0if (iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP,
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sc->sc_addr, &cmd, sizeof cmd, &=
data, sizeof data, 0)) {
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0iic=
_release_bus(sc->sc_tag, 0);
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0pri=
ntf(": cannot set temperature target register\n");
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
return;
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0}
> > >=20
> >=20
>=20