Hi Guenter, Like last time, I updated several files in my platform's v5.4 files and applied your three patches. >From what I can tell, I'm having no problems with the tmp461's basic support in the modified lm90 driver. Thanks again for looking into this, David From: Guenter Roeck <groeck7@xxxxxxxxx> on behalf of Guenter Roeck <linux@xxxxxxxxxxxx> Sent: Thursday, October 21, 2021 3:07 AM To: Wilson, David T. (GSFC-5870) <david.wilson@xxxxxxxx> Cc: linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; linux-hwmon@xxxxxxxxxxxxxxx <linux-hwmon@xxxxxxxxxxxxxxx> Subject: Re: [EXTERNAL] Re: Potential issue with smb word operations for tmp461 device in tmp401 driver Hi David, On 10/20/21 10:37 PM, Wilson, David T. (GSFC-5870) wrote: > Guenter, > > I updated several files and applied the lm90.c and tmp401.c patches. It should be noted that my target platform only supports v5.4, so > I had to update additional files (i.e. hwmon.c/.h) in order to successfully compile and apply the patch to lm90.c, which could suggest > that some of my findings may have been due to poor updating of files. > > In regards to testing, I got temp1_input and temp2_input values that look good from a glance and that steadily increased after booting > my target platform. There were three areas of possible concern. > 1. The lm90.c patch includes "+#define TMP461_REG_DFC 0x23". If this define is referring to the address of the digital filter control, > then the value should be 0x24, since 0x23 is the n-Factor Correction register Good catch. That is a bug. Thanks a lot for noticing. > 2. The values I recorded had a resolution of 0.125 deg resolution (e.g., 30625, 25250, etc...). However, the datasheet for the tmp461 > notes that the resolution is 0.0625 deg for local and remote channels, so I was wondering if that would also be supported in lm90.c You are correct, the lm90 driver assumes a 0.125 deg resolution. I'll see what I can do about that. > 3. For temp1_min and temp2_min, I used "cat temp1_min" to get the value -128000. Assuming that is -128.0 deg, I don't believe the > tmp461 should be returning a value underneath -64 deg based on the temp data format if I'm understanding the datasheet correctly. > It is odd, but the tmp461 datasheet does list a default value of 0x80 for the low limit registers. In 2-complement format, that does translate to -128 degrees C. That doesn't really make much sense, but it is what the chip reports, and I hesitate to change that. Thanks, Guenter > Thanks, > David > > From: Guenter Roeck <groeck7@xxxxxxxxx> on behalf of Guenter Roeck <linux@xxxxxxxxxxxx> > Sent: Tuesday, October 19, 2021 10:18 AM > To: Wilson, David T. (GSFC-5870) <david.wilson@xxxxxxxx> > Cc: linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; linux-hwmon@xxxxxxxxxxxxxxx <linux-hwmon@xxxxxxxxxxxxxxx> > Subject: Re: [EXTERNAL] Re: Potential issue with smb word operations for tmp461 device in tmp401 driver > > David, > > On 10/18/21 10:42 PM, Wilson, David T. (GSFC-5870) wrote: >> Guenter, >> >> I've tried testing the patch by checking out the most recent commit's version for tmp401.c and applying the patch. However, the temperature values seem to be to low <1000 for each read. >> >> If I'm understanding the driver code correctly, I believe there's two places of interest that might explain the low values >> 1. In tmp401_update_device_reg16, the final assignment to data->temp[j][i] overrides the patch's assignment to data->temp[j][i] in the tmp461 branch >> 2. In SENSOR_DEVICE_ATTR_2_RW, the temp2_offset is at (6, 1), but the newly added LSB array does not contain the LSB address at (6, 1) >> > Thanks a lot for the information. > >> Regarding your most recent email, I will try the lm90 by changing the driver tree entry to "ti,tmp451" and I'll let you know if that works for me. >> > I just sent two patches, one to remove tmp461 support from the tmp401 driver, and one to add > tmp461 support to the lm90 driver. Turns out tmp461 is almost identical to tmp451, which was > already supported by the lm90 driver. Adding support for tmp461 to the lm90 driver makes > much more sense than trying to fix the tmp401 driver. > > I module tested the lm90 driver for both tmp451 and tmp461 (I was able to find > register dumps for both chips), so I am reasonably sure that it works. > > Thanks, > Guenter > >> Thanks, >> David >> >> From: Guenter Roeck <groeck7@xxxxxxxxx> on behalf of Guenter Roeck <linux@xxxxxxxxxxxx> >> Sent: Tuesday, October 19, 2021 12:56 AM >> To: Wilson, David T. (GSFC-5870) <david.wilson@xxxxxxxx> >> Cc: linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>; linux-hwmon@xxxxxxxxxxxxxxx <linux-hwmon@xxxxxxxxxxxxxxx> >> Subject: [EXTERNAL] Re: Potential issue with smb word operations for tmp461 device in tmp401 driver >> >> David, >> >> On 10/15/21 10:43 AM, Wilson, David T. (GSFC-5870) wrote: >>> Hi, >>> >>> I am reporting what I believe is a potential issue in the tmp401 driver for the tmp461 device specifically. I am new to reporting issues, so I apologize in advance if I've provided insufficient information for an issue report. >>> >>> The problem I'm encountering is that when I use the tmp401 linux driver to read temperature values from the tmp461, all of the read temperature values end with 996 (e.g. 33996, 38996, etc...). >>> >>> Looking further into the tmp401 commit messages, I see that the driver was changed to use smb word operations instead of separate byte operations. Although the other supported devices (i.e. tmp432, etc...) are noted to support 16-bit read operations in their respective datasheets, I see no indications of 16-bit read support in the tmp461 datasheet, which is supported by my inquiry in the TI forums (https://gcc02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fe2e.ti.com%2Fsupport%2Fsensors-group%2Fsensors%2Ff%2Fsensors-forum%2F1044935%2Ftmp461-linux-driver-support-and-16-bit-temperature-register-reads&data=04%7C01%7Cdavid.wilson%40nasa.gov%7C2e3728d5f2e74810482108d994618683%7C7005d45845be48ae8140d43da96dd17b%7C0%7C0%7C637703968869859896%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3n9QPCDAzD0Qoy16KVtTzVNAe42KBgFdGbZr3naFWko%3D&reserved=0). >>> >>> Reverting the driver to the commit before the smb word change, I am then able to read temperature values that do not end only with 996. As a result, I believe that the tmp461 support may be partially broken by the switch to smb word operations. >>> >> >> can you try to instantiate the lm90 driver (instead of the tmp401 driver) >> and let me know if it works for you ? If your system uses devicetree, >> you might have to select "ti,tmp451" instead of "ti,tmp461". >> >> Thanks, >> Guenter >>
