Re: [PATCH 3/3] i2c: Add Intel USBIO I2C driver

Hans de Goede
2025-08-11T09:49:33Z

Hi Sakari,

Thank you for your review, note this
is not a full reply.

On 11-Aug-25 9:16 AM, Sakari Ailus wrote:

<snip>

>> +static int usbio_i2c_probe(struct auxiliary_device *adev,
>> + const struct auxiliary_device_id *adev_id)
>> +{
>> + struct usbio_i2c_bus_desc *i2c_desc;
>> + struct device *dev = &adev->dev;
>> + u8 dummy_read_buf;
>> + struct i2c_msg dummy_read = {
>> + .addr = 0x08,
>> + .flags = I2C_M_RD,
>> + .len = 1,
>> + .buf = &dummy_read_buf,
>> + };
>> + struct usbio_i2c *i2c;
>> + u32 max_speed;
>> + int ret;
>> +
>> + i2c_desc = dev_get_platdata(dev);
>> + if (!i2c_desc)
>> + return -EINVAL;
>> +
>> + /* Some USBIO chips have caps set to 0, but all chips can do 400KHz */
>> + if (!i2c_desc->caps)
>> + max_speed = I2C_MAX_FAST_MODE_FREQ;
>> + else
>> + max_speed = usbio_i2c_speeds[i2c_desc->caps & USBIO_I2C_BUS_MODE_CAP_MASK];
>> +
>> + i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
>> + if (!i2c)
>> + return -ENOMEM;
>
> Same comment on devm memory allocation than on the GPIO driver: I think you
> need to use the release callback of struct device here.

And more or less the same reply, i2c_del_adapter() ensures
all i2c_clients on the adapters bus are unregistered so
it guarantees that after i2c_del_adapter() none of the adapter
functions will get called.

So freeing the struct containing the adapter after remove()
has run is fine.

I know that the media subsystem does not handle v4l2-subdevs
(which the i2c-clients are) going away very well.

Richard mentioned that after a fw-update the usbio chip will
not come back until a reboot. And I've noticed that after crashing
the usbio fw it will not come back until a full power-cycle.

So we do not need to worry about somehow slotting new i2c-clients
into the media-controller graph after a disconnect + reconnect
since the reconnect will never happen during the current boot.

We do need to somehow make sure that trying to access the v4l2-subdev
after disconnect does not cause oopses or worse.

We will likely need to somehow keep the memory for the v4l2-subdev
around and add something like a disconnected / dead flag to it.

Regards,

Hans