Re: [PATCH v10 1/7] mfd: Add core driver for Nuvoton NCT6694
From: Lee Jones <lee@kernel.org>
Date: 2025-05-09 14:28:27
Also in:
linux-can, linux-gpio, linux-hwmon, linux-i2c, linux-rtc, linux-usb, linux-watchdog, lkml
On Fri, 02 May 2025, Ming Yu wrote:
Lee Jones [off-list ref] 於 2025年5月2日 週五 下午4:08寫道:quoted
...quoted
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),These are all identical. I thought you were going to use PLATFORM_DEVID_AUTO? In fact, you are already using PLATFORM_DEVID_AUTO since you are calling mfd_add_hotplug_devices(). So you don't need this IDs. MFD_CELL_NAME() should do.Yes, it uses PLATFORM_DEVID_AUTO, but in my implementation, the sub-devices use cell->id instead of platform_device->id, so it doesn't affect the current behavior. However, if you think there's a better approach or that this should be changed for consistency or correctness, I'm happy to update it, please let me know your recommendation. When using MFD_CELL_NAME(), the platform_device->id for the GPIO devices is assigned values from 1 to 16, and for the I2C devices from 1 to 6, but I need the ID offset to start from 0 instead.Oh no, don't do that. mfd_cell isn't supposed to be used outside of MFD. Just use the platform_device id-- if you really need to start from 0. As an aside, I'm surprised numbering starts from 1.OK, I will use platform_device->id instead. However, I'm still unsure why the ID starts from1. Additionally, I noticed that when calling mfd_add_devices() separately, the IDs are also assigned consecutively (e.g., GPIO: 1~16, I2C: 17~22, ...). Do you have any recommendations on how I should implement this?
If you are to use this mechanism, you'd have to submit separate mfd_add_devices() calls I guess. However, this all seems a bit silly for simple, contextless (where device 3 is identical to device 10, etc) enumeration. Can you use IDA instead? -- Lee Jones [李琼斯]