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 <vasconcel...@videotron.ca> 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. > > I'm not fluent in openfirmware but fussing there, I had an impression that > there could be a way there... > > Alternatively, maybe in Jon's machine case it's worth to check the following > project: > https://github.com/andreafabrizi/G4FanControl > > Cheers. > > Le samedi 25 novembre 2023 à 08:57 -0700, Theo de Raadt a écrit : > > For the temperature sensing drivers we have *intentionally* avoided doing > > writes to i2c devices. > > > > On many machines, ACPI/SMI retain control of these devices, and make > > changes behind OpenBSD's back without us knowing what is going on. We > > don't know if this will break a machine, do to their changes depending > > up on a specific configuration. The breakage could be subtle and very > > weird. 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. > > > > As a result, all sensor drivers for these chips only do 'sensing', > > meaning they only do reads. (OK, a few drivers do writes to "enable" > > 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). > > > > That said, your argument is for macppc. 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. > > > > jon@elytron.openbsd.amsterdam wrote: > > > > > 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. > > > > > > 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 > > > > > > 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. > > > > > > All the best > > > > > > Index: dev/i2c/adt7460.c > > > =================================================================== > > > 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 6 Apr 2022 18:59:28 -0000 1.22 > > > +++ dev/i2c/adt7460.c 23 Nov 2023 16:23:48 -0000 > > > @@ -34,6 +34,9 @@ > > > #define ADT7460_REM1_TEMP 0x25 > > > #define ADT7460_LOCAL_TEMP 0x26 > > > #define ADT7460_REM2_TEMP 0x27 > > > +#define ADT7460_REM1_TEMP_TRG 0x68 > > > +#define ADT7460_LOCAL_TEMP_TRG 0x67 > > > +#define ADT7460_REM2_TEMP_TRG 0x69 > > > #define ADT7460_TACH1L 0x28 > > > #define ADT7460_TACH1H 0x29 > > > #define ADT7460_TACH2L 0x2a > > > @@ -193,6 +196,16 @@ adt_attach(struct device *parent, struct > > > sc->sc_addr, &cmd, sizeof cmd, &data, sizeof data, > > > 0)) { > > > iic_release_bus(sc->sc_tag, 0); > > > printf(": cannot set control register\n"); > > > + return; > > > + } > > > + } > > > + > > > + for (cmd = ADT7460_LOCAL_TEMP_TRG; cmd <= ADT7460_REM2_TEMP_TRG; > > > cmd++){ > > > + data = 46; /* temperature target in deg C*/ > > > + if (iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP, > > > + sc->sc_addr, &cmd, sizeof cmd, &data, sizeof data, > > > 0)) { > > > + iic_release_bus(sc->sc_tag, 0); > > > + printf(": cannot set temperature target > > > register\n"); > > > return; > > > } > > > } > > > > > >
