[PATCH RFC] eeprom: ee1004: add support for temperature sensor

[PATCH RFC] eeprom: ee1004: add support for temperature sensor

[Heiner Kallweit]

Jean and me are discussing how to best instantiate temperature sensors
that can be found on RAM modules. First idea was to extend
i2c_register_spd() but I think reading the "temp sensor present" flag
from SPD can't be properly done from an i2c core level.
Therefore, for DDR4, do it from the ee1004 driver.

The temp sensor i2c address can be derived from the SPD i2c address,
so I think we can directly instantiate the device and don't have to
probe for it.
If the temp sensor has been instantiated already by other means
(e.g. class-based auto-detection), then the busy-check in 
i2c_new_client_device will detect this.

Link: https://www.spinics.net/lists/linux-i2c/msg65963.html
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index a1acd7713..4bce8f9d9 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -158,6 +158,22 @@ static struct bin_attribute *ee1004_attrs[] = {
 
 BIN_ATTRIBUTE_GROUPS(ee1004);
 
+static void ee1004_probe_temp_sensor(struct i2c_client *client)
+{
+	struct i2c_board_info info = { .type = "jc42" };
+	u8 byte14;
+	int ret;
+
+	/* byte 14, bit 7 is set if temp sensor is present */
+	ret = ee1004_eeprom_read(client, &byte14, 14, 1);
+	if (ret != 1 || !(byte14 & BIT(7)))
+		return;
+
+	info.addr = 0x18 | (client->addr & 7);
+
+	i2c_new_client_device(client->adapter, &info);
+}
+
 static void ee1004_cleanup(int idx)
 {
 	if (--ee1004_dev_count == 0)
@@ -204,6 +220,9 @@ static int ee1004_probe(struct i2c_client *client)
 		err = -EOPNOTSUPP;
 		goto err_clients;
 	}
+
+	ee1004_probe_temp_sensor(client);
+
 	mutex_unlock(&ee1004_bus_lock);
 
 	dev_info(&client->dev,
-- 
2.42.1

Re: [PATCH RFC] eeprom: ee1004: add support for temperature sensor

[kernel test robot]

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <c295881f-77aa-4f54-b06c-d48031d76339@gmail.com>
References: <c295881f-77aa-4f54-b06c-d48031d76339@gmail.com>
TO: Heiner Kallweit <hkallweit1@gmail.com>

Hi Heiner,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build warnings:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus soc/for-next linus/master v6.7-rc1 next-20231117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Heiner-Kallweit/eeprom-ee1004-add-support-for-temperature-sensor/20231115-191927
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/c295881f-77aa-4f54-b06c-d48031d76339%40gmail.com
patch subject: [PATCH RFC] eeprom: ee1004: add support for temperature sensor
:::::: branch date: 4 days ago
:::::: commit date: 4 days ago
config: i386-randconfig-141-20231119 (https://download.01.org/0day-ci/archive/20231119/202311192239.S2bnUkLI-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231119/202311192239.S2bnUkLI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202311192239.S2bnUkLI-lkp@intel.com/

smatch warnings:
drivers/misc/eeprom/ee1004.c:169 ee1004_probe_temp_sensor() error: uninitialized symbol 'byte14'.

vim +/byte14 +169 drivers/misc/eeprom/ee1004.c

b63866efa10ca5 Heiner Kallweit 2021-05-19  160  
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  161  static void ee1004_probe_temp_sensor(struct i2c_client *client)
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  162  {
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  163  	struct i2c_board_info info = { .type = "jc42" };
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  164  	u8 byte14;
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  165  	int ret;
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  166  
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  167  	/* byte 14, bit 7 is set if temp sensor is present */
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  168  	ret = ee1004_eeprom_read(client, &byte14, 14, 1);
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15 @169  	if (ret != 1 || !(byte14 & BIT(7)))
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  170  		return;
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  171  
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  172  	info.addr = 0x18 | (client->addr & 7);
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  173  
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  174  	i2c_new_client_device(client->adapter, &info);
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  175  }
4d88fe3c4e7dcf Heiner Kallweit 2023-11-15  176  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH RFC] eeprom: ee1004: add support for temperature sensor

[Heiner Kallweit]

On 15.11.2023 12:17, Heiner Kallweit wrote:
> Jean and me are discussing how to best instantiate temperature sensors
> that can be found on RAM modules. First idea was to extend
> i2c_register_spd() but I think reading the "temp sensor present" flag
> from SPD can't be properly done from an i2c core level.
> Therefore, for DDR4, do it from the ee1004 driver.
> 
> The temp sensor i2c address can be derived from the SPD i2c address,
> so I think we can directly instantiate the device and don't have to
> probe for it.
> If the temp sensor has been instantiated already by other means
> (e.g. class-based auto-detection), then the busy-check in 
> i2c_new_client_device will detect this.
> 
> Link: https://www.spinics.net/lists/linux-i2c/msg65963.html
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/misc/eeprom/ee1004.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
> index a1acd7713..4bce8f9d9 100644
> --- a/drivers/misc/eeprom/ee1004.c
> +++ b/drivers/misc/eeprom/ee1004.c
> @@ -158,6 +158,22 @@ static struct bin_attribute *ee1004_attrs[] = {
>  
>  BIN_ATTRIBUTE_GROUPS(ee1004);
>  
> +static void ee1004_probe_temp_sensor(struct i2c_client *client)
> +{
> +	struct i2c_board_info info = { .type = "jc42" };
> +	u8 byte14;
> +	int ret;
> +
> +	/* byte 14, bit 7 is set if temp sensor is present */
> +	ret = ee1004_eeprom_read(client, &byte14, 14, 1);
> +	if (ret != 1 || !(byte14 & BIT(7)))
> +		return;
> +
> +	info.addr = 0x18 | (client->addr & 7);
> +
> +	i2c_new_client_device(client->adapter, &info);
> +}
> +
>  static void ee1004_cleanup(int idx)
>  {
>  	if (--ee1004_dev_count == 0)
> @@ -204,6 +220,9 @@ static int ee1004_probe(struct i2c_client *client)
>  		err = -EOPNOTSUPP;
>  		goto err_clients;
>  	}
> +
> +	ee1004_probe_temp_sensor(client);
> +
>  	mutex_unlock(&ee1004_bus_lock);
>  
>  	dev_info(&client->dev,

Jean, do you have any feedback on this one?
Also not sure why this patch is marked "superseded" in patchwork.

Re: [PATCH RFC] eeprom: ee1004: add support for temperature sensor

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 148 bytes --]


> Also not sure why this patch is marked "superseded" in patchwork.

Mistake on my side. Should have been "RFC".

Now on to your other patches...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]