Linux I2C development
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Congatec Board Controller: Add storage devices support
@ 2026-08-11 16:15 Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle Thomas Richard (congatec GmbH)
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-08-11 16:15 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	Thomas Richard (congatec GmbH), stable, Sashiko

Second iteration and nothing fancy, I just fixed a typo in CGBC BIOS EEPROM
label, and added a patch to make cgbc_devs (MFD cells) const in MFD driver.

Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
Changes in v2:
- i2c: fix typo in CGBC BIOS EEPROM labeli.
- mfd: make cgbc_devs const
- Link to v1: https://patch.msgid.link/20260804-cgbc-i2c-storage-devices-support-v1-0-fed38510671c@bootlin.com

To: Thomas Richard <thomas.richard@bootlin.com>
To: Lee Jones <lee@kernel.org>
To: Andi Shyti <andi.shyti@kernel.org>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Werner Gartner <Werner.Gartner@congatec.com>
Cc: mfd@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: linux-i2c@vger.kernel.org

---
Thomas Richard (congatec GmbH) (5):
      mfd: cgbc: Fix use of negative error code as valid session handle
      mfd: cgbc: Make cgbc_devs const
      i2c: cgbc: Add virtual I2C bus support
      mfd: cgbc: Add virtual I2C bus support
      i2c: cgbc: Add virtual storage devices on the virtual I2C bus

 drivers/i2c/busses/i2c-cgbc.c | 174 ++++++++++++++++++++++++++++++++++++++++--
 drivers/mfd/cgbc-core.c       |   9 ++-
 2 files changed, 175 insertions(+), 8 deletions(-)
---
base-commit: 498c377a1ca412502649484c7b1788d5d328fc85
change-id: 20260717-cgbc-i2c-storage-devices-support-b3ee7cea3320

Best regards,
--  
Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle
  2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
@ 2026-08-11 16:15 ` Thomas Richard (congatec GmbH)
  2026-09-02 15:02   ` Lee Jones
  2026-08-11 16:15 ` [PATCH v2 2/5] mfd: cgbc: Make cgbc_devs const Thomas Richard (congatec GmbH)
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-08-11 16:15 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	Thomas Richard (congatec GmbH), stable, Sashiko

The cgbc_session_command() return value was directly cast to u8 and used
as session handle without error checking. Casting a negative error to u8
produces a valid-looking session handle. So check if return value is
positive before to cast and use it.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260713-cgbc-core-fix-cgbc-remove-v1-1-79274ad62b3a%40bootlin.com?part=1
Fixes: 6f1067cfbee7 ("mfd: Add Congatec Board Controller driver")
Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
 drivers/mfd/cgbc-core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
index 2becaf797646..4a409234e66c 100644
--- a/drivers/mfd/cgbc-core.c
+++ b/drivers/mfd/cgbc-core.c
@@ -103,7 +103,11 @@ static int cgbc_session_request(struct cgbc_device_data *cgbc)
 	if (ret)
 		return dev_err_probe(cgbc->dev, ret, "device not found or not ready\n");
 
-	cgbc->session = cgbc_session_command(cgbc, CGBC_SESSION_CMD_REQUEST);
+	ret = cgbc_session_command(cgbc, CGBC_SESSION_CMD_REQUEST);
+	if (ret < 0)
+		return dev_err_probe(cgbc->dev, ret, "session handle request timed out\n");
+
+	cgbc->session = ret;
 
 	/* The Board Controller sent us a wrong session handle, we cannot communicate with it */
 	if (cgbc->session < CGBC_SESSION_VALID_MIN || cgbc->session > CGBC_SESSION_VALID_MAX)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/5] mfd: cgbc: Make cgbc_devs const
  2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle Thomas Richard (congatec GmbH)
@ 2026-08-11 16:15 ` Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 3/5] i2c: cgbc: Add virtual I2C bus support Thomas Richard (congatec GmbH)
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-08-11 16:15 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	Thomas Richard (congatec GmbH)

Make cgbc_devs const, so it will be placed in .rodata section.

Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
 drivers/mfd/cgbc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
index 4a409234e66c..f09fd671bc8c 100644
--- a/drivers/mfd/cgbc-core.c
+++ b/drivers/mfd/cgbc-core.c
@@ -235,7 +235,7 @@ int cgbc_command(struct cgbc_device_data *cgbc, void *cmd, unsigned int cmd_size
 }
 EXPORT_SYMBOL_GPL(cgbc_command);
 
-static struct mfd_cell cgbc_devs[] = {
+static const struct mfd_cell cgbc_devs[] = {
 	{ .name = "cgbc-wdt"	},
 	{ .name = "cgbc-gpio"	},
 	{ .name = "cgbc-i2c", .id = 1 },

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 3/5] i2c: cgbc: Add virtual I2C bus support
  2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 2/5] mfd: cgbc: Make cgbc_devs const Thomas Richard (congatec GmbH)
@ 2026-08-11 16:15 ` Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 4/5] mfd: " Thomas Richard (congatec GmbH)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-08-11 16:15 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	Thomas Richard (congatec GmbH)

Internally the Board Controller has a virtual I2C bus with some virtual
storage devices on it. Add support for it by defining a i2c_adapter entry.
This virtual bus has a fixed frequency that cannot be changed.

Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
 drivers/i2c/busses/i2c-cgbc.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-cgbc.c b/drivers/i2c/busses/i2c-cgbc.c
index 25a74fa51aa0..c3f1c83b105a 100644
--- a/drivers/i2c/busses/i2c-cgbc.c
+++ b/drivers/i2c/busses/i2c-cgbc.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 
 #define CGBC_I2C_PRIMARY_BUS_ID	0
+#define CGBC_I2C_VIRTUAL_BUS_ID	3
 #define CGBC_I2C_PM_BUS_ID	4
 
 #define CGBC_I2C_CMD_START	0x40
@@ -147,11 +148,14 @@ static int cgbc_i2c_set_frequency(struct i2c_adapter *adap,
 	cmd[0] = CGBC_I2C_CMD_SPEED | algo_data->bus_id;
 	cmd[1] = cgbc_i2c_freq_to_reg(bus_frequency);
 
-	ret = cgbc_command(cgbc, &cmd, sizeof(cmd), &data, 1, NULL);
-	if (ret)
-		return dev_err_probe(i2c->dev, ret,
-				     "Failed to initialize I2C bus %s",
-				     adap->name);
+	/* Virtual bus has a fixed frequency */
+	if (algo_data->bus_id != CGBC_I2C_VIRTUAL_BUS_ID) {
+		ret = cgbc_command(cgbc, &cmd, sizeof(cmd), &data, 1, NULL);
+		if (ret)
+			return dev_err_probe(i2c->dev, ret,
+					     "Failed to initialize I2C bus %s",
+					     adap->name);
+	}
 
 	cmd[1] = 0x00;
 
@@ -338,6 +342,7 @@ static const struct i2c_algorithm cgbc_i2c_algorithm = {
 static struct i2c_algo_cgbc_data cgbc_i2c_algo_data[] = {
 	{ .bus_id = CGBC_I2C_PRIMARY_BUS_ID },
 	{ .bus_id = CGBC_I2C_PM_BUS_ID },
+	{ .bus_id = CGBC_I2C_VIRTUAL_BUS_ID },
 };
 
 static const struct i2c_adapter cgbc_i2c_adapter[] = {
@@ -357,6 +362,14 @@ static const struct i2c_adapter cgbc_i2c_adapter[] = {
 		.algo_data	= &cgbc_i2c_algo_data[1],
 		.nr		= -1,
 	},
+	{
+		.owner          = THIS_MODULE,
+		.name           = "Congatec Virtual I2C adapter",
+		.class          = I2C_CLASS_DEPRECATED,
+		.algo           = &cgbc_i2c_algorithm,
+		.algo_data      = &cgbc_i2c_algo_data[2],
+		.nr             = -1,
+	},
 };
 
 static int cgbc_i2c_probe(struct platform_device *pdev)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 4/5] mfd: cgbc: Add virtual I2C bus support
  2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
                   ` (2 preceding siblings ...)
  2026-08-11 16:15 ` [PATCH v2 3/5] i2c: cgbc: Add virtual I2C bus support Thomas Richard (congatec GmbH)
@ 2026-08-11 16:15 ` Thomas Richard (congatec GmbH)
  2026-08-11 16:15 ` [PATCH v2 5/5] i2c: cgbc: Add virtual storage devices on the virtual I2C bus Thomas Richard (congatec GmbH)
  2026-08-13 17:20 ` [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard
  5 siblings, 0 replies; 9+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-08-11 16:15 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	Thomas Richard (congatec GmbH)

Internally the Board Controller has a virtual I2C bus with some virtual
storage devices (EEPROM and RAM). So add a new I2C cell for this virtual
bus.

Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
 drivers/mfd/cgbc-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
index f09fd671bc8c..e800a0f4e50a 100644
--- a/drivers/mfd/cgbc-core.c
+++ b/drivers/mfd/cgbc-core.c
@@ -240,6 +240,7 @@ static const struct mfd_cell cgbc_devs[] = {
 	{ .name = "cgbc-gpio"	},
 	{ .name = "cgbc-i2c", .id = 1 },
 	{ .name = "cgbc-i2c", .id = 2 },
+	{ .name = "cgbc-i2c", .id = 3 },
 	{ .name = "cgbc-hwmon"	},
 	{ .name = "cgbc-backlight" },
 };

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 5/5] i2c: cgbc: Add virtual storage devices on the virtual I2C bus
  2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
                   ` (3 preceding siblings ...)
  2026-08-11 16:15 ` [PATCH v2 4/5] mfd: " Thomas Richard (congatec GmbH)
@ 2026-08-11 16:15 ` Thomas Richard (congatec GmbH)
  2026-08-13 17:20 ` [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard
  5 siblings, 0 replies; 9+ messages in thread
From: Thomas Richard (congatec GmbH) @ 2026-08-11 16:15 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	Thomas Richard (congatec GmbH)

Add support for virtual storage devices exposed by the Board Controller
on its virtual I2C bus. There are two device types: EEPROM for persistent
storage and RAM for non-persistent storage:
- Secure Data EEPROM  (64 bytes, RO): static and dynamic board info
- BIOS EEPROM         (32 bytes, RW): reserved for BIOS applications
- BC EEPROM           (32 bytes, RO): Board Controller operational params
- User EEPROM         (32 bytes, RW): user applications
- BIOS RAM            (32 bytes, RW): reserved for BIOS applications
- BC RAM               (8 bytes, RO): Board Controller operational params
- User RAM            (16 bytes, RW): user applications

Use the at24 driver with the 24c01 entry (the most generic one) for all
virtual storage devices, customizing parameters via software_node
properties (size, pagesize, read-only flag, label).

Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
---
 drivers/i2c/busses/i2c-cgbc.c | 151 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 150 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-cgbc.c b/drivers/i2c/busses/i2c-cgbc.c
index c3f1c83b105a..e36755cc963b 100644
--- a/drivers/i2c/busses/i2c-cgbc.c
+++ b/drivers/i2c/busses/i2c-cgbc.c
@@ -372,6 +372,149 @@ static const struct i2c_adapter cgbc_i2c_adapter[] = {
 	},
 };
 
+static const struct property_entry cgbc_secure_data_eeprom_props[] = {
+	PROPERTY_ENTRY_U32("size", 64),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_BOOL("read-only"),
+	PROPERTY_ENTRY_STRING("label", "cgbc-secure-data-eeprom"),
+	{ }
+};
+
+static const struct software_node cgbc_secure_data_eeprom_node = {
+	.properties = cgbc_secure_data_eeprom_props,
+};
+
+static const struct property_entry cgbc_bc_eeprom_props[] = {
+	PROPERTY_ENTRY_U32("size", 32),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_BOOL("read-only"),
+	PROPERTY_ENTRY_STRING("label", "cgbc-bc-eeprom"),
+	{ }
+};
+
+static const struct software_node cgbc_bc_eeprom_node = {
+	.properties = cgbc_bc_eeprom_props,
+};
+
+static const struct property_entry cgbc_user_eeprom_props[] = {
+	PROPERTY_ENTRY_U32("size", 32),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_STRING("label", "cgbc-user-eeprom"),
+	{ }
+};
+
+static const struct software_node cgbc_user_eeprom_node = {
+	.properties = cgbc_user_eeprom_props,
+};
+
+static const struct property_entry cgbc_bios_eeprom_props[] = {
+	PROPERTY_ENTRY_U32("size", 32),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_STRING("label", "cgbc-bios-eeprom"),
+	{ }
+};
+
+static const struct software_node cgbc_bios_eeprom_node = {
+	.properties = cgbc_bios_eeprom_props,
+};
+
+static const struct property_entry cgbc_bc_ram_props[] = {
+	PROPERTY_ENTRY_U32("size", 8),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_BOOL("read-only"),
+	PROPERTY_ENTRY_STRING("label", "cgbc-bc-ram"),
+	{ }
+};
+
+static const struct software_node cgbc_bc_ram_node = {
+	.properties = cgbc_bc_ram_props,
+};
+
+static const struct property_entry cgbc_user_ram_props[] = {
+	PROPERTY_ENTRY_U32("size", 16),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_STRING("label", "cgbc-user-ram"),
+	{ }
+};
+
+static const struct software_node cgbc_user_ram_node = {
+	.properties = cgbc_user_ram_props,
+};
+
+static const struct property_entry cgbc_bios_ram_props[] = {
+	PROPERTY_ENTRY_U32("size", 32),
+	PROPERTY_ENTRY_U32("pagesize", 1),
+	PROPERTY_ENTRY_STRING("label", "cgbc-bios-ram"),
+	{ }
+};
+
+static const struct software_node cgbc_bios_ram_node = {
+	.properties = cgbc_bios_ram_props,
+};
+
+static const struct i2c_board_info cgbc_i2c_board_info[] = {
+	{
+		.type = "24c01",
+		.addr = 0x40,
+		.dev_name = "cgbc-secure-data-eeprom",
+		.swnode = &cgbc_secure_data_eeprom_node,
+	},
+	{
+		.type = "24c01",
+		.addr = 0x48,
+		.dev_name = "cgbc-bc-eeprom",
+		.swnode = &cgbc_bc_eeprom_node,
+	},
+	{
+		.type = "24c01",
+		.addr = 0x50,
+		.dev_name = "cgbc-user-eeprom",
+		.swnode = &cgbc_user_eeprom_node,
+	},
+	{
+		.type = "24c01",
+		.addr = 0x58,
+		.dev_name = "cgbc-bios-eeprom",
+		.swnode = &cgbc_bios_eeprom_node,
+	},
+	{
+		.type = "24c01",
+		.addr = 0x60,
+		.dev_name = "cgbc-bc-ram",
+		.swnode = &cgbc_bc_ram_node,
+	},
+	{
+		.type = "24c01",
+		.addr = 0x68,
+		.dev_name = "cgbc-user-ram",
+		.swnode = &cgbc_user_ram_node,
+	},
+	{
+		.type = "24c01",
+		.addr = 0x70,
+		.dev_name = "cgbc-bios-ram",
+		.swnode = &cgbc_bios_ram_node,
+	}
+};
+
+static void cgbc_i2c_instantiate_devices(struct i2c_adapter *adap)
+{
+	struct i2c_algo_cgbc_data *algo_data = adap->algo_data;
+	struct i2c_client *client;
+	int i;
+
+	/* We only instantiate devices on Virtual bus */
+	if (algo_data->bus_id != CGBC_I2C_VIRTUAL_BUS_ID)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(cgbc_i2c_board_info); i++) {
+		client  = i2c_new_client_device(adap, &cgbc_i2c_board_info[i]);
+		if (IS_ERR(client))
+			dev_err(&adap->dev, "Failed to register %s\n",
+				cgbc_i2c_board_info[i].dev_name);
+	}
+}
+
 static int cgbc_i2c_probe(struct platform_device *pdev)
 {
 	struct cgbc_device_data *cgbc = dev_get_drvdata(pdev->dev.parent);
@@ -393,7 +536,13 @@ static int cgbc_i2c_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	return i2c_add_numbered_adapter(&i2c->adap);
+	ret = i2c_add_numbered_adapter(&i2c->adap);
+	if (ret)
+		return ret;
+
+	cgbc_i2c_instantiate_devices(&i2c->adap);
+
+	return ret;
 }
 
 static void cgbc_i2c_remove(struct platform_device *pdev)

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 0/5] Congatec Board Controller: Add storage devices support
  2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
                   ` (4 preceding siblings ...)
  2026-08-11 16:15 ` [PATCH v2 5/5] i2c: cgbc: Add virtual storage devices on the virtual I2C bus Thomas Richard (congatec GmbH)
@ 2026-08-13 17:20 ` Thomas Richard
  5 siblings, 0 replies; 9+ messages in thread
From: Thomas Richard @ 2026-08-13 17:20 UTC (permalink / raw)
  To: Lee Jones, Andi Shyti
  Cc: Thomas Petazzoni, Werner Gartner, mfd, linux-kernel, linux-i2c,
	stable, Sashiko

On 8/11/26 6:15 PM, Thomas Richard (congatec GmbH) wrote:
> Second iteration and nothing fancy, I just fixed a typo in CGBC BIOS EEPROM
> label, and added a patch to make cgbc_devs (MFD cells) const in MFD driver.
Please ignore this version. Following the discussion with Lee on
hard-coding the cell ID, I'm finally preparing a new version of this
series which switches to PLATFORM_DEVID_AUTO before to add the virtual
bus and virtual storage devices support.

Also IMO having the i2c_board_info struct in the I2C bus driver is a bit
odd. The I2C devices topology on the virtual bus can be different,
depending the Board Controller. I think this complexity shall be handled
on MFD side.

At the end I should have a more generic I2C bus driver, and
platform_data will be used to pass all parameters (including I2C devices
to register on the I2C bus) to the I2C bus driver.

Best Regards,
Thomas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle
  2026-08-11 16:15 ` [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle Thomas Richard (congatec GmbH)
@ 2026-09-02 15:02   ` Lee Jones
  2026-09-02 15:03     ` Lee Jones
  0 siblings, 1 reply; 9+ messages in thread
From: Lee Jones @ 2026-09-02 15:02 UTC (permalink / raw)
  To: Thomas Richard (congatec GmbH)
  Cc: Andi Shyti, Thomas Petazzoni, Werner Gartner, mfd, linux-kernel,
	linux-i2c, stable, Sashiko

On Tue, 11 Aug 2026, Thomas Richard (congatec GmbH) wrote:

> The cgbc_session_command() return value was directly cast to u8 and used
> as session handle without error checking. Casting a negative error to u8
> produces a valid-looking session handle. So check if return value is
> positive before to cast and use it.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260713-cgbc-core-fix-cgbc-remove-v1-1-79274ad62b3a%40bootlin.com?part=1
> Fixes: 6f1067cfbee7 ("mfd: Add Congatec Board Controller driver")
> Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
> ---
>  drivers/mfd/cgbc-core.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
> index 2becaf797646..4a409234e66c 100644
> --- a/drivers/mfd/cgbc-core.c
> +++ b/drivers/mfd/cgbc-core.c
> @@ -103,7 +103,11 @@ static int cgbc_session_request(struct cgbc_device_data *cgbc)
>  	if (ret)
>  		return dev_err_probe(cgbc->dev, ret, "device not found or not ready\n");
>  
> -	cgbc->session = cgbc_session_command(cgbc, CGBC_SESSION_CMD_REQUEST);
> +	ret = cgbc_session_command(cgbc, CGBC_SESSION_CMD_REQUEST);
> +	if (ret < 0)
> +		return dev_err_probe(cgbc->dev, ret, "session handle request timed out\n");

Can we make this a bit more user friendly?

"Failed to start XYZ session"?

> +
> +	cgbc->session = ret;
>  
>  	/* The Board Controller sent us a wrong session handle, we cannot communicate with it */
>  	if (cgbc->session < CGBC_SESSION_VALID_MIN || cgbc->session > CGBC_SESSION_VALID_MAX)
> 
> -- 
> 2.53.0
> 

-- 
Lee Jones

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle
  2026-09-02 15:02   ` Lee Jones
@ 2026-09-02 15:03     ` Lee Jones
  0 siblings, 0 replies; 9+ messages in thread
From: Lee Jones @ 2026-09-02 15:03 UTC (permalink / raw)
  To: Thomas Richard (congatec GmbH)
  Cc: Andi Shyti, Thomas Petazzoni, Werner Gartner, mfd, linux-kernel,
	linux-i2c, stable, Sashiko

On Wed, 02 Sep 2026, Lee Jones wrote:

> On Tue, 11 Aug 2026, Thomas Richard (congatec GmbH) wrote:
> 
> > The cgbc_session_command() return value was directly cast to u8 and used
> > as session handle without error checking. Casting a negative error to u8
> > produces a valid-looking session handle. So check if return value is
> > positive before to cast and use it.
> > 
> > Cc: stable@vger.kernel.org
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/#/patchset/20260713-cgbc-core-fix-cgbc-remove-v1-1-79274ad62b3a%40bootlin.com?part=1
> > Fixes: 6f1067cfbee7 ("mfd: Add Congatec Board Controller driver")
> > Signed-off-by: Thomas Richard (congatec GmbH) <thomas.richard@bootlin.com>
> > ---
> >  drivers/mfd/cgbc-core.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
> > index 2becaf797646..4a409234e66c 100644
> > --- a/drivers/mfd/cgbc-core.c
> > +++ b/drivers/mfd/cgbc-core.c
> > @@ -103,7 +103,11 @@ static int cgbc_session_request(struct cgbc_device_data *cgbc)
> >  	if (ret)
> >  		return dev_err_probe(cgbc->dev, ret, "device not found or not ready\n");
> >  
> > -	cgbc->session = cgbc_session_command(cgbc, CGBC_SESSION_CMD_REQUEST);
> > +	ret = cgbc_session_command(cgbc, CGBC_SESSION_CMD_REQUEST);
> > +	if (ret < 0)
> > +		return dev_err_probe(cgbc->dev, ret, "session handle request timed out\n");
> 
> Can we make this a bit more user friendly?
> 
> "Failed to start XYZ session"?

The other two MFD patches are fine.

-- 
Lee Jones

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-02 15:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-11 16:15 [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard (congatec GmbH)
2026-08-11 16:15 ` [PATCH v2 1/5] mfd: cgbc: Fix use of negative error code as valid session handle Thomas Richard (congatec GmbH)
2026-09-02 15:02   ` Lee Jones
2026-09-02 15:03     ` Lee Jones
2026-08-11 16:15 ` [PATCH v2 2/5] mfd: cgbc: Make cgbc_devs const Thomas Richard (congatec GmbH)
2026-08-11 16:15 ` [PATCH v2 3/5] i2c: cgbc: Add virtual I2C bus support Thomas Richard (congatec GmbH)
2026-08-11 16:15 ` [PATCH v2 4/5] mfd: " Thomas Richard (congatec GmbH)
2026-08-11 16:15 ` [PATCH v2 5/5] i2c: cgbc: Add virtual storage devices on the virtual I2C bus Thomas Richard (congatec GmbH)
2026-08-13 17:20 ` [PATCH v2 0/5] Congatec Board Controller: Add storage devices support Thomas Richard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox