Hi all. I've been trying to spend some cycles thinking about the IRQ acquisition
refactor, following up on 93b6604c5a66 ("i2c: Allow recovery of the initial IRQ
by an I2C client device."), and wanted to run some ideas by you. Apologies if
there's already something in the works.
My first thought was that we could add a get_irq function pointer to the client
that, if non-null, would be called during probe to obtain the IRQ. Initially,
the i2c_new_device function would probably need to determine which get_irq
function to use. It would have logic similar to that existing in
i2c_device_probe. This change would be moving in the wrong direction in that
regard, but the idea would be that the this could eventually be specified in
i2_board_info.
The probe function would become:
static int i2c_device_probe(struct device *dev)
{
/* ... */
client->irq = 0;
if (client->get_irq && !driver->disable_i2c_core_irq_mapping) {
int irq = client->get_irq();
if (irq == -EPROBE_DEFER)
return irq;
if (irq > 0)
client->irq = irq;
}
/* ... */
}
Many callers to i2c_device_probe allocate an i2c_board_info on a stack and only
populate type, addr, and platform_data. For these, the function pointer would
be NULL, so no IRQ acquisition would be attempted. In this case, maybe we could
introduce a i2c_new_device_simple to make use of these devices easier and help
identify many of the devices that don't need IRQs?
Most of the original users of i2c_board_info structure (hard coded platform
definitions) register those structures. In this case, it should be possible
for a get_irq function variant to look those up at probe time to obtain
resources.
The get_irq function for device tree case would be a simple variation of
the existing code block in the probe function.
For ACPI case, the walk that currenty finds GPIO IRQ types would need to be
modified to identify IRQ and extended IRQ types. Maybe the existing
i2c_acpi_get_info could be exposed for this.
One case that concerns me a bit is I2C_CLIENT_HOST_NOTIFY. The flag is not set
or cleared in the client after initialization in any in-tree usages, but is it
safe to assume that it never will be? If that is the case, the get_irq function
could just be i2c_smbus_host_notify_to_irq.
A catch-all for other cases could copy the resources, or maybe the
entire board info. And, of course, other get_irq functions could be added.
A second option would be to provide get_info and release_info functions. This
would be more flexible and allow more of the initialization to be moved out
of i2c_new_device. It could also allow the client to see some changes
without reprobing, if that's desirable.
The code in the probe would look something like this:
static int i2c_device_probe(struct device *dev)
{
i2c_board_info *info = NULL;
/* ... */
if (client->get_board_info)
info = client->get_board_info();
client->irq = 0;
if (info && !driver->disable_i2c_core_irq_mapping) {
int irq = get_irq(client, info);
if (irq == -EPROBE_DEFER) {
if (client->release_board_info)
client->release_board_info (info);
return irq;
}
if (irq > 0)
client->irq = irq;
}
if (client->release_board_info)
client->release_board_info (info);
/* ... */
}
static int get_i2_device_irq(struct i2c_client *client,
struct i2c_board_info const *info)
{
if (client->flags & I2C_CLIENT_HOST_NOTIFY) {
dev_dbg(dev, "Using Host Notify IRQ\n");
return i2c_smbus_host_notify_to_irq(client);
} else if (info->irq)
return info->irq;
else
return i2c_dev_irq_from_resources(info->resources,
info->num_resources);
return -ENOENT;
}
The get_board_info function variations are similar to the get_irq functions
above. In ACPI and device-tree cases, the board info structures can be
re-created at any time. Those board info structures that are registered can
be looked up and the release function pointer would be null or no-op. Other
cases would copy the board info structure in i2c_new_device.
A drawback of this approach is that we would need to copy the board info
structure into the client in many cases where devices don't care about IRQs
(the "simple" case mentioned above).
Finally, there is an option to copy more data from the board info structure
in the i2c_new_device function. This is obviously the simplest and least
invasive, but also duplicates data in many cases and allocates memory that
isn't used in several others.
I was initially leaning towards the first option, but as I'm writing this, I
can see more benefits of the second. The third could certainly be a step
in the direction one of the other two options since there are cases where
we need to copy data to the client in all three strategies. What do you think?
Thank you,
Jim
