[...]

quoted

quoted

quoted

quoted

quoted

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));
...

No, that's clearly way worse.  =:-)

The clean-up that this provides is probably not worth all of this
discussion.  I _still_ think this enumeration should be done in the
driver.  But if you really can't make it work, I'll accept the .id
patch.

Okay, I would like to ask for your advice regarding the implementation of IDA.

Using a global IDA in the sub-driver like this:
(in i2c-nct6694.c)
static DEFINE_IDA(nct6694_i2c_ida);

static int nct6694_i2c_probe(struct platform_device *pdev)
{
    ida_alloc(&nct6694_i2c_ida, GFP_KERNEL);
    ...
}

causes IDs to be globally incremented across all devices. For example,
the first NCT6694 device gets probed 6 times and receives IDs 0–5, but
when a second NCT6694 device is added, it receives IDs starting from
6, rather than starting again from 0. This makes per-device ID mapping
unreliable.

To solve this, I believe the right approach is to have each NCT6694
instance maintain its own IDA, managed by the MFD driver's private
data. As mentioned earlier, for example:
(in nct6694.c)
struct nct6694 {
    struct device *dev;
    struct ida i2c_ida;
};

static int nct6694_probe(struct platform_device *pdev)
{
    ...
    ida_init(&nct6694->i2c_ida);
    ...
}

(in i2c-nct6694.c)
static int nct6694_i2c_probe(struct platform_device *pdev)
{
    id = ida_alloc(&nct6694->i2c_ida, GFP_KERNEL);
}

This way, each device allocates IDs independently, and each set of
I2C/GPIO instances gets predictable IDs starting from 0 per device. I
think this resolves the original issue without relying on hardcoded
platform IDs.
Please let me know if this implementation aligns with what you had in mind.

This sounds like an acceptable way forward.

-- 
Lee Jones [李琼斯]