On Tue, 14 Nov 2023 15:44:36 +0100, Heiner Kallweit wrote: > On 14.11.2023 15:00, Jean Delvare wrote: > > On Wed, 08 Nov 2023 11:28:45 +0100, Heiner Kallweit wrote: > >> From a maintenance point of view the easiest solution would be: > >> - set flag I2C_CLASS_DEPRECATED in addition to I2C_CLASS_SPD > >> to encourage potential users to switch to explicit instantiating > > > > Bad idea. That's just going to spam a warning message on millions of > > systems while there's just nothing most users can do about it. That's > > not helpful, we are already aware of the problem, and we are the guys > > looking into it. > > I'm afraid I wasn't precise enough when writing this. What I meant is > adding I2C_CLASS_DEPRECATED for the mux'ed child segments in i801. > So it should affect users of the few Asus systems only. Oh right, should have been obvious actually. Nevertheless, my point still stands, the purpose of I2C_CLASS_DEPRECATED is to draw the attention of the developers, and in this case, the developers would be us, so we are already aware. > i2c_register_spd() isn't used there, so I'd assume these users don't > miss the temp sensors on their RAM modules. i2c_register_spd() only deals with SPD EEPROMs. The fact that it isn't called for mux'd Asus boards only means that the users have to manually declare the at24 devices from user-space (which is a regression from earlier when just loading the eeprom driver would automatically create the devices). Users of these boards can still load the jc42 driver and the thermal sensors on the memory modules will be supported, as long as I2C_CLASS_SPD exists and is used by both i2c-i801 (for the mux'd segments) and the jc42 driver. > > (...) > > I think the steps to follow are: > > * Extend i2c_register_spd() to support up to 8 memory modules (I'm > > already working on that, patch is coming). > > * Call i2c_register_spd() on the mux'd SMBus segments on the Asus > > boards. > > * Extend i2c_register_spd() to also instantiate jc42 devices in > > addition to at24 (or ee1004) devices. I think this is better than > > writing a separate function as I initially suggested. The reason why > > I think so is because the SPD EEPROM does contain the information > > about thermal sensor presence. So the code which instantiates the at24 > > or ee1004 device could also read from it to figure out whether a jc42 > > device must be instantiated. This removes the need for probing. > > I miss some insight here on which type of memory modules we can expect > jc42-4 compatible temp sensors. I saw DDR3 mentioned (including LPDDR3?), The LP prefix is for low-power. For most practical purposes, on the software side, you can consider these modules as equivalent to their standard version, and treat them alike. > not sure about DDR4. In case of DDR4 we would have to read the EE1004 > data structure to check the "temp sensor present" bit. So I wonder > whether instantiating the temp sensor should be in ee1004 driver. I wasn't sure so I looked up the technical literature. The Microchip MCP98244 datasheet mentions use on a DDR4 memory module, and the NXP SE97B and SE98A datasheet mentions use on DDR2 and DDR3 modules. -- Jean Delvare SUSE L3 Support
