linux-kernel - [RFT PATCH v2 3/3] hwmon: (spd5118) Add support for Renesas/ITD SPD5118 hub controllers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240618195348.1670547-4-linux@roeck-us.net>
Date: Tue, 18 Jun 2024 12:53:48 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: linux-hwmon@...r.kernel.org
Cc: linux-kernel@...r.kernel.org,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	René Rebe <rene@...ctcode.de>,
	Thomas Weißschuh <linux@...ssschuh.net>,
	Armin Wolf <W_Armin@....de>,
	Stephen Horvath <s.horvath@...look.com.au>,
	Paul Menzel <pmenzel@...gen.mpg.de>,
	Sasha Kozachuk <skozachuk@...gle.com>,
	John Hamrick <johnham@...gle.com>,
	Chris Sarra <chrissarra@...gle.com>,
	Guenter Roeck <linux@...ck-us.net>
Subject: [RFT PATCH v2 3/3] hwmon: (spd5118) Add support for Renesas/ITD SPD5118 hub controllers

The SPD5118 specification says, in its documentation of the page bits
in the MR11 register:

"
This register only applies to non-volatile memory (1024) Bytes) access of
SPD5 Hub device.
For volatile memory access, this register must be programmed to '000'.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"

Renesas/ITD SPD5118 hub controllers take this literally and disable access
to volatile memory if the page selected in MR11 is != 0. Since the BIOS or
ROMMON will access the non-volatile memory and likely select a page != 0,
this means that the driver will not instantiate since it can not identify
the chip. Even if the driver instantiates, access to volatile registers
is blocked after a nvram read operation which selects a page other than 0.

To solve the problem, add initialization code to select page 0 during
probe. Before doing that, use basic validation to ensure that this is
really a SPD5118 device and not some random EEPROM.

Cc: Sasha Kozachuk <skozachuk@...gle.com>
Cc: John Hamrick <johnham@...gle.com>
Cc: Chris Sarra <chrissarra@...gle.com>
Signed-off-by: Guenter Roeck <linux@...ck-us.net>
---
v2: Simplified to use mechanisms introduced in the first two patches
    of the series.

 drivers/hwmon/spd5118.c | 56 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 995c45e2a997..80a02dba2ccc 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -538,6 +538,58 @@ static const struct regmap_config spd5118_regmap_config = {
 	.num_ranges = ARRAY_SIZE(spd5118_regmap_range_cfg),
 };
 
+static int spd5118_init(struct i2c_client *client)
+{
+	struct i2c_adapter *adapter = client->adapter;
+	int err, regval, mode;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
+	if (regval < 0 || (regval && regval != 0x5118))
+		return -ENODEV;
+
+	/*
+	 * If the device type registers return 0, it is possible that the chip
+	 * has a non-zero page selected and takes the specification literally,
+	 * i.e. disables access to volatile registers besides the page register
+	 * if the page is not 0. Try to identify such chips.
+	 */
+	if (!regval) {
+		/* Vendor ID registers must also be 0 */
+		regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR);
+		if (regval)
+			return -ENODEV;
+
+		/* The selected page in MR11 must not be 0 */
+		mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
+		if (mode < 0 || (mode & ~SPD5118_LEGACY_MODE_MASK) ||
+		    !(mode & SPD5118_LEGACY_PAGE_MASK))
+			return -ENODEV;
+
+		err = i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE,
+						mode & SPD5118_LEGACY_MODE_ADDR);
+		if (err)
+			return -ENODEV;
+
+		/*
+		 * If the device type registers are still bad after selecting
+		 * page 0, this is not a SPD5118 device. Restore original
+		 * legacy mode register value and abort.
+		 */
+		regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
+		if (regval != 0x5118) {
+			i2c_smbus_write_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE, mode);
+			return -ENODEV;
+		}
+	}
+
+	/* We are reasonably sure that this is really a SPD5118 hub controller */
+	return 0;
+}
+
 static int spd5118_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -547,6 +599,10 @@ static int spd5118_probe(struct i2c_client *client)
 	struct regmap *regmap;
 	int err;
 
+	err = spd5118_init(client);
+	if (err)
+		return err;
+
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-- 
2.39.2