Re: [PATCH 3/4] mfd: cgbc: Add virtual I2C bus support

On 8/12/26 1:20 PM, Lee Jones wrote:
> On Tue, 04 Aug 2026, [email protected] wrote:
> 
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Low] Hard-coded cell IDs are used for the newly added `cgbc-i2c` subdevice instead of `PLATFORM_DEVID_AUTO`.
>> --
>>
>> --- Patch [3]: [PATCH 3/4] mfd: cgbc: Add virtual I2C bus support ---
>> commit 4f0038d5dc42bfd45540675cdbf89f68ab3673d5
>> Author: Thomas Richard (congatec GmbH) <[email protected]>
>>
>> mfd: cgbc: Add virtual I2C bus support
>>
>> This commit adds a new I2C cell to support the internal virtual I2C bus 
>> provided by the Board Controller.
>>
>>> diff --git a/drivers/mfd/cgbc-core.c b/drivers/mfd/cgbc-core.c
>>> index 4a409234e66c8..81bd0a381ca13 100644
>>> --- a/drivers/mfd/cgbc-core.c
>>> +++ b/drivers/mfd/cgbc-core.c
>>> @@ -240,6 +240,7 @@ static struct mfd_cell cgbc_devs[] = {
>>>  	{ .name = "cgbc-gpio"	},
>>>  	{ .name = "cgbc-i2c", .id = 1 },
>>>  	{ .name = "cgbc-i2c", .id = 2 },
>>> +	{ .name = "cgbc-i2c", .id = 3 },
>>
>> [Severity: Low]
>> Should this use PLATFORM_DEVID_AUTO instead of hard-coding the cell ID?
> 

I use mfd_add_devices() with PLATFORM_DEVID_NONE.
So I2C busses will have platform_id from 0 to 2 [1].
And I2C adapter is selected using the platform device id [2].

I'm not 100% convinced by this implementation. I planned to add new I2C
busses and new boards support, and each board selects some busses. I
guess it will be the right time to rework it. If you have any idea to
implement this in a better way, please let me know.

[1]
https://elixir.bootlin.com/linux/v7.2-rc7/source/drivers/mfd/mfd-core.c#L164-L167
[2]
https://elixir.bootlin.com/linux/v7.2-rc7/source/drivers/i2c/busses/i2c-cgbc.c#L374


Best Regards,
Thomas