Re: [PATCH v12 1/7] mfd: Add core driver for Nuvoton NCT6694
From: Guenter Roeck
Date: Thu Jun 19 2025 - 12:58:30 EST
On 6/19/25 09:20, Greg KH wrote:
On Fri, Jun 20, 2025 at 12:03:01AM +0800, Ming Yu wrote:
Lee Jones <lee@xxxxxxxxxx> 於 2025年6月19日 週四 下午11:28寫道：
On Thu, 19 Jun 2025, Ming Yu wrote:
Lee Jones <lee@xxxxxxxxxx> 於 2025年6月19日 週四 下午7:53寫道：
On Fri, 13 Jun 2025, Ming Yu wrote:
Lee Jones <lee@xxxxxxxxxx> 於 2025年6月13日 週五 下午9:11寫道：
On Fri, 13 Jun 2025, Ming Yu wrote:
Lee Jones <lee@xxxxxxxxxx> 於 2025年6月12日 週四 下午11:23寫道：
On Thu, 12 Jun 2025, Ming Yu wrote:
Dear Lee,
Thank you for reviewing,
Lee Jones <lee@xxxxxxxxxx> 於 2025年6月12日 週四 下午10:00寫道：
...
+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?
What is wrong with increasing ids? The id value means nothing, they
just have to be unique.
Unless they are used in the client driver as index into an array, as in
"this is the Nth instance of this device for this chip". There has to be
_some_ means to pass N to the client driver.
Guenter