Dear Greg and Lee,

Thank you for your comments.
I've reviewed your suggestions, but would appreciate your feedback on
a few remaining points.

Lee Jones [off-list ref] 於 2025年6月25日 週三 下午5:01寫道：

On Fri, 20 Jun 2025, Ming Yu wrote:

quoted

Lee Jones [off-list ref] 於 2025年6月19日 週四 下午11:28寫道：

quoted

On Thu, 19 Jun 2025, Ming Yu wrote:

quoted

Lee Jones [off-list ref] 於 2025年6月19日 週四 下午7:53寫道：

quoted

On Fri, 13 Jun 2025, Ming Yu wrote:

quoted

Lee Jones [off-list ref] 於 2025年6月13日 週五 下午9:11寫道：

quoted

On Fri, 13 Jun 2025, Ming Yu wrote:

quoted

Lee Jones [off-list ref] 於 2025年6月12日 週四 下午11:23寫道：

quoted

On Thu, 12 Jun 2025, Ming Yu wrote:

quoted

Dear Lee,

Thank you for reviewing,

Lee Jones [off-list ref] 於 2025年6月12日 週四 下午10:00寫道：

quoted

...

quoted

quoted

+static const struct mfd_cell nct6694_devs[] = {
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 0),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 1),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 2),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 3),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 4),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 5),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 6),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 7),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 8),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 9),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 10),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 11),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 12),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 13),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 14),
+     MFD_CELL_BASIC("nct6694-gpio", NULL, NULL, 0, 15),
+
+     MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 0),
+     MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 1),
+     MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 2),
+     MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 3),
+     MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 4),
+     MFD_CELL_BASIC("nct6694-i2c", NULL, NULL, 0, 5),

Why have we gone back to this silly numbering scheme?

What happened to using IDA in the child driver?

In a previous version, I tried to maintain a static IDA in each
sub-driver. However, I didn’t consider the case where multiple NCT6694
devices are bound to the same driver — in that case, the IDs are not
fixed and become unusable for my purpose.

Not sure I understand.

As far as I know, if I maintain the IDA in the sub-drivers and use
multiple MFD_CELL_NAME("nct6694-gpio") entries in the MFD, the first
NCT6694 device bound to the GPIO driver will receive IDs 0~15.
However, when a second NCT6694 device is connected to the system, it
will receive IDs 16~31.
Because of this behavior, I switched back to using platform_device->id.

Each of the devices will probe once.

The first one will be given 0, the second will be given 1, etc.

Why would you give multiple IDs to a single device bound to a driver?

The device exposes multiple peripherals — 16 GPIO controllers, 6 I2C
adapters, 2 CAN FD controllers, and 2 watchdog timers. Each peripheral
is independently addressable, has its own register region, and can
operate in isolation. The IDs are used to distinguish between these
instances.
For example, the GPIO driver will be probed 16 times, allocating 16
separate gpio_chip instances to control 8 GPIO lines each.

If another device binds to this driver, it is expected to expose
peripherals with the same structure and behavior.

I still don't see why having a per-device IDA wouldn't render each
probed device with its own ID.  Just as you have above.

For example, when the MFD driver and the I2C sub-driver are loaded,
connecting the first NCT6694 USB device to the system results in 6
nct6694-i2c platform devices being created and bound to the
i2c-nct6694 driver. These devices receive IDs 0 through 5 via the IDA.

However, when a second NCT6694 USB device is connected, its
corresponding nct6694-i2c platform devices receive IDs 6 through 11 —
instead of 0 through 5 as I originally expected.

If I've misunderstood something, please feel free to correct me. Thank you!

In the code above you register 6 I2C devices.  Each device will be
assigned a platform ID 0 through 5. The .probe() function in the I2C
driver will be executed 6 times.  In each of those calls to .probe(),
instead of pre-allocating a contiguous assignment of IDs here, you
should be able to use IDA in .probe() to allocate those same device IDs
0 through 5.

What am I missing here?

You're absolutely right in the scenario where a single NCT6694 device
is present. However, I’m wondering how we should handle the case where
a second or even third NCT6694 device is bound to the same MFD driver.
In that situation, the sub-drivers using a static IDA will continue
allocating increasing IDs, rather than restarting from 0 for each
device. How should this be handled?

I'd like to see the implementation of this before advising.

In such a case, I assume there would be a differentiating factor between
the two (or three) devices.  You would then use that to decide which IDA
would need to be incremented.

However, Greg is correct.  Hard-coding look-ups for userspace to use
sounds like a terrible idea.

I understand.
Do you think it would be better to pass the index via platform_data
and use PLATFORM_DEVID_AUTO together with mfd_add_hotplug_devices()
instead?
For example:
struct nct6694_platform_data {
    int index;
};

static struct nct6694_platform_data i2c_data[] = {
    { .index = 0 }, { .index = 1 }, { .index = 2 }, { .index = 3 }, {
.index = 4 }, { .index = 5 },
};

static const struct mfd_cell nct6694_devs[] = {
    MFD_CELL_BASIC("nct6694-i2c", NULL, &i2c_data[0], sizeof(struct
nct6694_platform_data), 0),
    MFD_CELL_BASIC("nct6694-i2c", NULL, &i2c_data[1], sizeof(struct
nct6694_platform_data), 0),
    MFD_CELL_BASIC("nct6694-i2c", NULL, &i2c_data[2], sizeof(struct
nct6694_platform_data), 0),
    MFD_CELL_BASIC("nct6694-i2c", NULL, &i2c_data[3], sizeof(struct
nct6694_platform_data), 0),
    MFD_CELL_BASIC("nct6694-i2c", NULL, &i2c_data[4], sizeof(struct
nct6694_platform_data), 0),
    MFD_CELL_BASIC("nct6694-i2c", NULL, &i2c_data[5], sizeof(struct
nct6694_platform_data), 0),
};
...
mfd_add_hotplug_devices(dev, nct6694_devs, ARRAY_SIZE(nct6694_devs));
...

Thank you again for your support.


Best regards,
Ming